Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BufferImpl, UndoTree, LineReaderImpl excessive memory usage #477

Closed
ProIcons opened this issue Nov 23, 2019 · 9 comments
Closed

BufferImpl, UndoTree, LineReaderImpl excessive memory usage #477

ProIcons opened this issue Nov 23, 2019 · 9 comments
Milestone

Comments

@ProIcons
Copy link

So i've been thinking to create a public accessible cli service with Spring Shell that uses JLine 3.4 under the hood, and i was experimenting with some stuff, and then i tried to check if it can crash somehow, so i got a really huge string and provided it as input, the result was 8GB used ram, (from 176 mb) and a crash because java ran out of heap space.

I couldn't pinpoint the exact location of the issue, so i ran YourKit Java Profiler, got a memory snapshot, and i saw huge Usage on an int[] and on 4 classes allocating 2.4GB each, LineReaderImpl UndoTree UndoTree$Node and BufferImpl

Is that something that can be avoided? haven't studied the JLine's codebase. Just asking though,

@gnodet
Copy link
Member

gnodet commented Nov 24, 2019

It seems the UndoTree is not cleared or limited in its depth, so I think it grows without limit. That would explain what you see.
If you use a public accessible service, I would suggest to use a LineReader per user session, so that the lifecycle of a single LineReader would be limited.

@mattirn
Copy link
Collaborator

mattirn commented Nov 24, 2019

BufferImpl is simple class to test also alone. Reading a string of 72MB to buffer consumes about 537MB memory in BufferImpl class. String is memorized in int[] array.
So if you put a string of 176MB to BufferImpl it will consume memory about 1.3GB. Other three classes you mentioned have buffer as class field so their memory consumption is at least the same order.
Got a bit smaller estimation of memory consumption as your 2.4GB but we have not taken account how memory allocation is done in BufferImpl and where exactly a crash happened.

BufferImpl

@ProIcons
Copy link
Author

ProIcons commented Nov 24, 2019

Clarification, my input string was not 176 MB

I inputed a 88KB single line random string, on Intellij Idea's Dummy Terminal
And it crashed.

I used this https://github.com/dmadunic/clidemo project for testing,

out.log

Tested on OpenJDK 13, and OracleJDK 12

@mattirn
Copy link
Collaborator

mattirn commented Nov 24, 2019

You are using JLine 3.4, right?. From log I see that crash happens on line Buffer copy = buf.copy(); when calling BufferImpl copy() method.
The copy() method has been improved after JLine version 3.4. May be that is why I'm not able to reproduce it with the latest JLine version.

@ProIcons
Copy link
Author

<dependencyManagement>
        <dependencies>
            <dependency>
                <groupId>org.jline</groupId>
                <artifactId>jline</artifactId>
                <version>3.13.2-SNAPSHOT</version>
            </dependency>
            <dependency>
                <groupId>org.jline</groupId>
                <artifactId>jline-terminal-jna</artifactId>
                <version>3.13.2-SNAPSHOT</version>
            </dependency>
        </dependencies>
    </dependencyManagement>
2019-11-25 00:18:44.544 ERROR 6060 --- [           main] o.s.boot.SpringApplication               : Application run failed

java.lang.OutOfMemoryError: Java heap space
	at java.base/java.lang.Object.clone(Native Method)
	at org.jline.reader.impl.BufferImpl.<init>(BufferImpl.java:43)
	at org.jline.reader.impl.BufferImpl.copy(BufferImpl.java:49)
	at org.jline.reader.impl.BufferImpl.copy(BufferImpl.java:22)
	at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:703)
	at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:476)
	at org.springframework.shell.jline.InteractiveShellApplicationRunner$JLineInputProvider.readInput(InteractiveShellApplicationRunner.java:115)
	at org.springframework.shell.Shell.run(Shell.java:129)
	at org.springframework.shell.jline.InteractiveShellApplicationRunner.run(InteractiveShellApplicationRunner.java:84)
	at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:772)
	at org.springframework.boot.SpringApplication.callRunners(SpringApplication.java:762)
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:319)
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1215)
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:1204)
	at com.ag04.clidemo.ClidemoApplication.main(ClidemoApplication.java:11)


Process finished with exit code 1

@ProIcons
Copy link
Author

ProIcons commented Nov 24, 2019

When i ran it on a non Dummy Terminal.
I got this:

java.lang.OutOfMemoryError: Java heap space
        at java.base/java.util.Arrays.copyOfRange(Arrays.java:4103)
        at org.jline.utils.AttributedStringBuilder.subSequence(AttributedStringBuilder.java:85)
        at org.jline.utils.AttributedCharSequence.substring(AttributedCharSequence.java:251)
        at org.jline.utils.AttributedCharSequence.toAttributedString(AttributedCharSequence.java:356)
        at org.jline.reader.impl.LineReaderImpl.insertSecondaryPrompts(LineReaderImpl.java:4296)
        at org.jline.reader.impl.LineReaderImpl.insertSecondaryPrompts(LineReaderImpl.java:4233)
        at org.jline.reader.impl.LineReaderImpl.getDisplayedBufferWithPrompts(LineReaderImpl.java:4062)
        at org.jline.reader.impl.LineReaderImpl.redisplay(LineReaderImpl.java:3925)
        at org.jline.reader.impl.LineReaderImpl.redisplay(LineReaderImpl.java:3861)
        at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:733)
        at org.jline.reader.impl.LineReaderImpl.readLine(LineReaderImpl.java:476)
        at org.springframework.shell.jline.InteractiveShellApplicationRunner$JLineInputProvider.readInput(InteractiveShellApplicationRunner.java:115)
        at org.springframework.shell.Shell.run(Shell.java:129)
        at org.springframework.shell.jline.InteractiveShellApplicationRunner.run(InteractiveShellApplicationRunner.java:84)
        at org.springframework.boot.SpringApplication.callRunner(SpringApplication.java:772)
        at org.springframework.boot.SpringApplication.callRunners(SpringApplication.java:762)
        at org.springframework.boot.SpringApplication.run(SpringApplication.java:319)
        at org.springframework.boot.SpringApplication.run(SpringApplication.java:1215)
        at org.springframework.boot.SpringApplication.run(SpringApplication.java:1204)
        at com.ag04.clidemo.ClidemoApplication.main(ClidemoApplication.java:11)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)            at java.base/java.lang.reflect.Method.invoke(Method.java:567)
        at org.springframework.boot.maven.AbstractRunMojo$LaunchRunner.run(AbstractRunMojo.java:496)
        at java.base/java.lang.Thread.run(Thread.java:835)

This time compiled with Oracle JDK 1.8

@gnodet
Copy link
Member

gnodet commented Nov 25, 2019

What can easily be done is to bypass a bunch of advanced features (undo, highlighting, parsing...) when the string is longer than a configurable length (with a default to 1000 characters for example).
This will help for CPU and memory usage as pasting a very long string makes the CLI unusable. I'll raise an issue for that. Note that the paste operation itself can be very long too and the redisplay too, so there are rooms for improvements in those 2 areas too. I think the buffer should be limited in length (configurable) to reject pasting huge strings.

@ProIcons
Copy link
Author

Let me check it and i will get back to you soon

@ProIcons
Copy link
Author

ProIcons commented Nov 26, 2019

It seems that fixed the issue Thanks for that, though there's some kind of an issue printing the prompt twice.
CLI-DEMO:>CLI-DEMO:> But it seems it is only happening on Dummy Terminals so.. low priority i guess? :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants