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

Extreme memory usage with large blocks of text #627

Closed
Ansury opened this issue Oct 22, 2017 · 21 comments
Closed

Extreme memory usage with large blocks of text #627

Ansury opened this issue Oct 22, 2017 · 21 comments

Comments

@Ansury
Copy link

Ansury commented Oct 22, 2017

Is it normal/expected to see this kind of memory usage? Memory is shown at the top of the screencaps.

The first one is a modified version of one of the demos which adds 4,000 lines. Memory usage on that one spikes to about twice what's shown here, and then drops down. It also functions fine.

rtfx1

This second screenshot is with 400,000 extra rows of text added. Demo becomes unusable with this much text. When I add a single letter to a row, it freezes for a few seconds before showing the character. Scrolling still works okay, but the rendering speed isn't great. Memory usage... 2GB? ❗ ❗ ❗ It also fluctuates wildly (up to 1GB or so) when scrolling, which is only going to make things worse.

rtfx2

I made every line different on purpose since it looks like some optimizations are going on under the hood, but even without that I still see this behavior.

I'm running on Linux. Oracle JRE 8u144

@JordanMartinez
Copy link
Contributor

Could you provide the code you used to produce this?

@garybentley
Copy link

garybentley commented Oct 25, 2017

Not sure what's going on with the OP's test. I used the LineIndicatorDemo and pasted ALL of the text from: http://www.gutenberg.org/cache/epub/1661/pg1661.txt until I got over 50,000 lines. Memory usage, as reported by the Windows Task Manager, started at 80MB with 13,000 lines and went up to 205MB with 52,000 lines (after manually scrolling/paging through every line). I didn't notice a drop in performance with scrolling or paging.

One thing to note however is that as you page up and down through the text the memory usage never decreases, it will constantly, slowly go up. Not that this means that much with the garbage collector and heap management, the memory manager may just be keeping that memory around but not used.

edit: I left the demo running with the 52,000 lines and the memory usage, after rising to 245MB has now fallen to 213MB so the garbage collector is cleaning things up.

@Ansury
Copy link
Author

Ansury commented Oct 25, 2017

This was about 400,000 lines, so 245/213MB sounds about in line with what I'm seeing (maybe even high). I'd be willing to bet that the garbage collector is the source of the (or much of the) performance issue, but memory usage shouldn't be this high in the first place. When I scroll, memory usage goes up and down pretty wildly. I suspect when you up the data, it will go down also like I'm seeing. Maybe it's not getting high enough in your test to bother with GC.

I think I basically just cloned the ContentScalabilityTest and added some text generation. I originally noticed the memory issue loading an 8MB text file, which came to about 400,000 rows of text.

I reduced the number of characters it adds per line but it still behaves the same.

Please don't be a Linux-only issue.Please don't be a Linux-only issue.Please don't be a Linux-only issue.Please don't be a Linux-only issue.Please don't be a Linux-only issue. 😅😅

package org.fxmisc.richtext.demo;

import java.text.NumberFormat;
import java.util.Collection;
import java.util.Collections;
import java.util.Timer;
import java.util.TimerTask;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.fxmisc.flowless.VirtualizedScrollPane;
import org.fxmisc.richtext.CodeArea;
import org.fxmisc.richtext.LineNumberFactory;
import org.fxmisc.richtext.model.StyleSpans;
import org.fxmisc.richtext.model.StyleSpansBuilder;

import javafx.application.Application;
import javafx.application.Platform;
import javafx.scene.Scene;
import javafx.scene.control.Label;
import javafx.scene.layout.StackPane;
import javafx.scene.layout.VBox;
import javafx.stage.Stage;

public class ScalabilityTest extends Application {

    private static final String[] KEYWORDS = new String[] {
            "abstract", "assert", "boolean", "break", "byte",
            "case", "catch", "char", "class", "const",
            "continue", "default", "do", "double", "else",
            "enum", "extends", "final", "finally", "float",
            "for", "goto", "if", "implements", "import",
            "instanceof", "int", "interface", "long", "native",
            "new", "package", "private", "protected", "public",
            "return", "short", "static", "strictfp", "super",
            "switch", "synchronized", "this", "throw", "throws",
            "transient", "try", "void", "volatile", "while"
    };

    private static final String KEYWORD_PATTERN = "\\b(" + String.join("|", KEYWORDS) + ")\\b";
    private static final String PAREN_PATTERN = "\\(|\\)";
    private static final String BRACE_PATTERN = "\\{|\\}";
    private static final String BRACKET_PATTERN = "\\[|\\]";
    private static final String SEMICOLON_PATTERN = "\\;";
    private static final String STRING_PATTERN = "\"([^\"\\\\]|\\\\.)*\"";
    private static final String COMMENT_PATTERN = "//[^\n]*" + "|" + "/\\*(.|\\R)*?\\*/";

    private static final Pattern PATTERN = Pattern.compile(
            "(?<KEYWORD>" + KEYWORD_PATTERN + ")"
            + "|(?<PAREN>" + PAREN_PATTERN + ")"
            + "|(?<BRACE>" + BRACE_PATTERN + ")"
            + "|(?<BRACKET>" + BRACKET_PATTERN + ")"
            + "|(?<SEMICOLON>" + SEMICOLON_PATTERN + ")"
            + "|(?<STRING>" + STRING_PATTERN + ")"
            + "|(?<COMMENT>" + COMMENT_PATTERN + ")"
    );

    private final int TEST_ROWS = 400 * 1000;
    private String sampleCode;
    private final Runtime runtime = Runtime.getRuntime();
    private Timer updateTimer;

    public static void main(String[] args) {
    	System.setProperty("prism.lcdtext", "false");
    	System.setProperty("prism.text", "t2k");
        launch(args);
    }

    public ScalabilityTest() {
    	StringBuilder sb = new StringBuilder(String.join("\n", new String[] {
    	        "package com.example;",
    	        "",
    	        "import java.util.*;",
    	        "",
    	        "public class Foo extends Bar implements Baz {",
    	        "",
    	        "    /*",
    	        "     * multi-line comment",
    	        "     */",
    	        "    public static void main(String[] args) {",
    	        "        // single-line comment",
    	        "        for(String arg: args) {",
    	        "            if(arg.length() != 0)",
    	        "                System.out.println(arg);",
    	        "            else",
    	        "                System.err.println(\"Warning: empty string as argument\");",
    	        "        }",
    	        "\n"
    	        }));
    	
    	for (int i=0 ; i < TEST_ROWS ; i++) {
    		sb.append("        System.out.println(\"");
    		sb.append(Math.random());
    		sb.append("\");\n");
    	}
    	
    	sb.append("    }\n");
    	sb.append("}\n");
    	sampleCode = sb.toString();
    }
    
    @Override
    public void start(Stage primaryStage) {
        CodeArea codeArea = new CodeArea();
        codeArea.setParagraphGraphicFactory(LineNumberFactory.get(codeArea));

        codeArea.richChanges()
                .filter(ch -> !ch.getInserted().equals(ch.getRemoved())) // XXX
                .subscribe(change -> {
                    codeArea.setStyleSpans(0, computeHighlighting(codeArea.getText()));
                });
        codeArea.replaceText(0, 0, sampleCode);

        VBox vBox = new VBox();
        Label memlbl = new Label();
        StackPane p = new StackPane(new VirtualizedScrollPane<>(codeArea));
        p.setPrefSize(1200, 880);
        vBox.getChildren().add(memlbl);
        vBox.getChildren().add(p);
        Scene scene = new Scene(vBox, 1200, 900);
        scene.getStylesheets().add(JavaKeywordsAsync.class.getResource("java-keywords.css").toExternalForm());
        primaryStage.setScene(scene);
        primaryStage.setTitle("CodeArea Scalability Test");
        primaryStage.show();
        
        updateTimer = new Timer(true);
        updateTimer.schedule(new TimerTask() {
            @Override
            public void run() {
                Platform.runLater(() -> {
                	memlbl.setText(memoryStatus());
                });
            }
        }, 0, 1000);
    }
    
    public String memoryStatus() {
        NumberFormat format = NumberFormat.getInstance();
        StringBuilder sb = new StringBuilder();
        long maxMemory = runtime.maxMemory();
        long allocatedMemory = runtime.totalMemory();
        long freeMemory = runtime.freeMemory();
        sb.append("Used memory: ");
        sb.append(format.format((runtime.totalMemory() - runtime.freeMemory()) / 1024));
        sb.append("KB  Avail memory: ");
        sb.append(format.format((freeMemory + (maxMemory - allocatedMemory)) / 1024));
        sb.append("KB");
        return sb.toString();
    }

    private static StyleSpans<Collection<String>> computeHighlighting(String text) {
        Matcher matcher = PATTERN.matcher(text);
        int lastKwEnd = 0;
        StyleSpansBuilder<Collection<String>> spansBuilder
                = new StyleSpansBuilder<>();
        while(matcher.find()) {
            String styleClass =
                    matcher.group("KEYWORD") != null ? "keyword" :
                    matcher.group("PAREN") != null ? "paren" :
                    matcher.group("BRACE") != null ? "brace" :
                    matcher.group("BRACKET") != null ? "bracket" :
                    matcher.group("SEMICOLON") != null ? "semicolon" :
                    matcher.group("STRING") != null ? "string" :
                    matcher.group("COMMENT") != null ? "comment" :
                    null; /* never happens */ assert styleClass != null;
            spansBuilder.add(Collections.emptyList(), matcher.start() - lastKwEnd);
            spansBuilder.add(Collections.singleton(styleClass), matcher.end() - matcher.start());
            lastKwEnd = matcher.end();
        }
        spansBuilder.add(Collections.emptyList(), text.length() - lastKwEnd);
        return spansBuilder.create();
    }
}

@Ansury
Copy link
Author

Ansury commented Oct 25, 2017

I'll add one thing here which may quickly explain this when we look into it closer (no time right now).

I thought I had tested this before, but I just tried disabling the codeArea.richChanges() block of code in start() which calls the computeHighlighting method. This reduced memory usage drastically, still maybe a bit high (I see 500MB now), but the test code itself might be causing some of the issue. Maybe some optimizations in the test itself are needed. Performance was fine after disabling that richChanges() block.

@garybentley
Copy link

Yep sorry, I missed the extra zero. Just tried the test again with 500,000 lines and memory is up to 675MB. Performance seems to be ok, everything is responsive.

@Ansury
Copy link
Author

Ansury commented Oct 26, 2017

That number seems a little low, but that's waaaaay higher than I'd expect for what we're doing here. That's ~over 1MB per 1,000 lines of text.

Not sure how much RAM you're working with but if you get it up to between 1.0GB and 1.5GB (like that test code above does for me), I think you'll start running into what is likely GC pauses.

@garybentley
Copy link

I'm not sure what number you would expect though. 1.3KB per line seems ok to me given the number of objects involved. Interestingly, I left my test running overnight and in the morning the memory usage was down to 4MB, so clearly the actual usage is far less and the GC isn't needing to clean things up immediately. I haven't been able to get a test over 1GB on Windows, I can't get gradle to allow the JavaExec task to have enough memory (I don't know gradle very well).

As a comparison, I use the Atom editor for my code editing (sorry RichTextFX) and created a buffer with 480,000 lines, the memory usage is now up to 1.7GB and isn't dropping. When I try and get to 500,000 lines it crashes.

@JordanMartinez
Copy link
Contributor

I use Linux, so running this on Linux Mint, I noticed:

  • my memory usage was ~1,200,000 KB (>1 GB) after it first rendered the area. Scrolling up/down would remove about another 100,000 - 200,000 KB from that initial amount. When I showed the top of the viewport, it reduced to 800,000 - 900,000 KB.
  • commenting out the system properties you have made the memory usage a 100k to 200k KB worse.
  • scrolling worked fine for the most part
  • using page up / page down would sometimes take a while to render. Pressing either one of those multiple times would cause the area to lag for a bit.
  • typing multiple letters caused the area to freeze for a good while before I killed it after 5 seconds or so. No doubt, this is due to not including the line .successionEnds(Duration.ofMillis(500)) after richChanges() and before the .filter() part. Thus, the syntax highlighting is being recalculated after every keystroke rather than after every period of user inactivity (500 ms after user stops typing). Even when I added that line, the area still lagged for a bit when I typed (not too surprising...)

Please don't be a Linux-only issue. Please don't be a Linux-only issue. Please don't be a Linux-only issue. Please don't be a Linux-only issue. Please don't be a Linux-only issue. 😅😅

Unfortunately, this seems to be a Linux-only issue...

@JordanMartinez
Copy link
Contributor

Removing the richChanges() code block reduced memory usage to ~420,000 KB because the StyleSpans objects created in that block are no longer created, nor rendered to the screen. Calling area.setStyleSpans(0, length(), computHighlighting(area.getText()) caused the memory to jump from ~488,000 KB to ~908,000 KB.

@Ansury
Copy link
Author

Ansury commented Oct 28, 2017

@garybentley Well, this tells us that Atom/Electron are also a pig:pig:...? :satisfied: (I guess in its defense, it's not meant to load a 500k line file, being a code editor.)

Here's the thing, and I know that under the hood we have alot going on object creation wise, but that 400,000 line file was only 8MB of actual data. For editing a file of that size to require well over 1GB of RAM... something incredibly wasteful is going on. I mean, is every glyph getting an object? There isn't anything inherently wrong with making use of memory like that (these days) when the usual data set is small, but it might mean that some alternate architecture is needed for editing large files.

For the heck of it I tried opening a structured text 8MB file with Kate text editor (Linux).
Obviously, they're doing things very differently... after turning on highlighting and scrolling all over the place, 186MB was the highest I saw. Not an apples to apples comparison, but I don't think we should be 10x that.

@JordanMartinez Yep, I noticed the same with removing richChanges(), and pretty similar on the other points. Damn...

So I guess I have a few options:
-Use something else for large file display/editing
-Go without highlighting and just put up with a bit higher memory use on Linux
-Try and find some way to optimize/workaround/fix. If this is Linux only I seriously doubt the cause is in this repository...

@JordanMartinez
Copy link
Contributor

If I'd have to guess, I'd say that some of the memory usage stems from a single paragraph's segment styles and segments being stored twice. Paragraph has a list of segments and a StyleSpans object (list of styles). However, the data in these two forms does not help a TextFlow render them since a single Text object must be used for each style/segment difference:

Segments: text text text text text
Styles:   ssssssszzzzbbbbbcccccccc
Result:   |     |   |    |       | // 4 Text objects to render this correctly

Thus, it creates a List<StyledSegment<SEG, S>> and TextFlow uses that to render it. Since a Paragraph is sometimes created without being rendered due to being an immutable object, we do not create this list object until it's needed. Then, once the result is calculated, it is stored until the paragraph is GC'd. Not a problem for most use cases but I can see why scaling that could become a problem.

However, since removing the richChange() block reduced the memory usage by almost half, most of the issue is due to creating many StyleSpans objects that aren't ever used when rendering the paragraphs in the viewport. So, if you could find a way to clear out all StyleSpans objects in the area and only set the styles of the paragraphs that are currently being rendered, it would save a good portion of that high memory usage.

This means the richChanges() block would need to be handled differently. Since I don't think you are using "context aware" syntax highlighting (e.g. different-colored braces/brackets so that the left one corresponds to the right one and is distinguishable from other braces/brackets), perhaps the code could be changed to something like this?

int previousStyleStart = 0;
int previousStyleEnd = area.getLength();
// something like this? ReactFX handles list changes
//   a bit differently, so I can't recall the exact API
EventStreams.changesOf(area.visibleParagraphs())
    .subscribe(change -> {
        // clear only the area where styles spans was applied previously
        area.clearStyles(previousStyleStart, previousStyleEnd);
        // calculate the style spans for just these visible paragraphs
        StyleSpans<S> visibleStyles = calculateSyntaxHighlighting();
        // find the start of where to apply them
        int firstVisibleIndex = area.getFirstVisibleParIndex();
        int start = area.position(firstVisibleIndex, 0).toOffset();
        // set the new style spans
        area.setStyleSpans(start, visibleStyles);
        // store the new previous style start/end for later clearing
        previousStyleStart = start;
        previousStyleEnd = start + visibleStyles.length();
    }

Also, perhaps using a different way to calculate the syntax highlighting other than a Matcher object might help.

@JordanMartinez
Copy link
Contributor

One other side note... Memory usage could be optimized if a Paragraph was refactored to no longer store styles in the object but instead only store segments and if the StyleSpans objects were stored in the StyledDocument (i.e. EditableStyledDocument / ReadOnlyStyledDocument). In cases where numerous styles are used throughout the area, the memory usage is no different than what we have now, though perhaps it might be a bit less. However, in cases where the styles span a large amount of length, less objects are needed to store the same information. Then, when rendering a paragraph, either the StyledDocument could handle the necessary calculations to recombine the styles and segments from a style-less paragraph into a different Paragraph type that TextFlowExt uses to render onto the screen (caching similar to what is already done in Paragraph now could also occur here) or TextFlowExt could itself handle this calculation entirely.

@JordanMartinez
Copy link
Contributor

Also, I believe ReactFX's FingerTree is only a 2-3 FingerTree. One other person commented that memory usage could be improved if it was changed into a 2-3-4 FingerTree. It sounded like supporting that was trivial but hadn't been done because it would require writing a lot of "boilerplate code" based on what's already there and just make it work for a 4-FingerTree.

@Ansury
Copy link
Author

Ansury commented Oct 30, 2017

Thanks for looking into that. I'll spend some time looking at this next weekend or so.

@JordanMartinez
Copy link
Contributor

Also, do we know if this high memory usage also occurs on Mac? Does it only affect Linux?

@Ansury
Copy link
Author

Ansury commented Nov 1, 2017

The only crApple product I own is a forgotten original iPod. 😝
(Sorry, trying to cut back on the Apple bashing some, but it's hard...)

But it would be interesting to know.

@JordanMartinez
Copy link
Contributor

On Linux, removing the richChanges() code block reduced memory usage to ~420,000 KB because the StyleSpans objects created in that block are no longer created, nor rendered to the screen. Calling area.setStyleSpans(0, length(), computHighlighting(area.getText()) caused the memory to jump from ~488,000 KB to ~908,000 KB.

I've tested the code again but using a commit that occurred before I decoupled the style from its segment. Here's the results on Linux:

  • After the initialization when styles are not being computed, it uses ~403,000 KB. As I scroll around, it uses up more memory. When it gets GC'd (around 480,000 KB of used memory for me), it drops to ~380,000 KB.
  • After initialization with styles being computed, it's ~970,000 KB but then it jumped up to 1,200,000 KB. When it GC'd, it dropped to 1.1 GB

To say the least, it seems like decoupling the style from its segment has come at the cost of performance to a degree. I think this could be minimized by making the StyledDocument store the styles as opposed to the Paragraphs. However, this still seems to be a Linux-specific issue since the same code on Windows apparently takes less.

@JordanMartinez
Copy link
Contributor

Here's a way to style only the visible text in an efficient manner. The only flaw is something in Flowless, which seems to change its scrolling values whenever it lays out its content:

SuspendableYes allowRichChange = new SuspendableYes();
EventStream<?> plainTextChange = codeArea.richChanges()
        .hook(c -> System.out.println("rich change occurred, firing event"))
        .successionEnds(Duration.ofMillis(500))
        .hook(c -> System.out.println("user has not typed anything for the given duration: firing event"))
        .conditionOn(allowRichChange)
        .hook(c -> System.out.println("this is a valid user-intitiated event, not us changing the style; so allow it to pass through"))
        .filter(ch -> !ch.isIdentity())
        .hook(c -> System.out.println("this is a valid plain text change. firing plaintext change event"));
EventStream<?> dirtyViewport = EventStreams
        .merge(
            codeArea.estimatedScrollXProperty().values(),
            codeArea.estimatedScrollYProperty().values())
        .hook(e -> System.out.println("y or x property value changed"))
        .successionEnds(Duration.ofMillis(200))
        .hook(e -> System.out.println("We've waited long enough, now fire an event"));
EventStreams.merge(plainTextChange, dirtyViewport)
        .hook(c -> System.out.println("either the viewport has changed or a plain text change has occurred. fire an event"))
        .subscribe(dirtyStyles -> {
            // rather than clearing the previously-visible text's styles and setting the current
            // visible text's styles....
            //
            //  codeArea.clearStyle(startOfLastVisibleTextStyle, endOfLastVisibleTextStyle);
            //  codeArea.setStyleSpans(offsetIntoContentBeforeReachingCurrentVisibleStyles, newStyles);
            //
            // do this entire action in one move
            //
            //  codeArea.setStyleSpans(0, styleSpans)
            //
            // where `styleSpans` parameters are...
            //  | unstyled | previously styled | currently visible text | previously styled | unstyled |
            //  | empty list                   | computeHighlighting()  | empty list                   |


            // compute the styles for the currently visible text
            StyleSpans<Collection<String>> visibleTextStyles = computeHighlighting(getVisibleText());

            // calculate how far into the content is the first part of the visible text
            // before modifying the area's styles
            int firstVisibleParIdx = codeArea.visibleParToAllParIndex(0);
            int startOfVisibleStyles = codeArea.position(firstVisibleParIdx, 0).toOffset();
            int lengthFollowingVisibleStyles = codeArea.getLength() - startOfVisibleStyles - visibleTextStyles.length();

            StyleSpans<Collection<String>> styleSpans = visibleTextStyles
                    // 1 single empty list before visible styles
                    .prepend(new StyleSpan<>(Collections.emptyList(), startOfVisibleStyles))
                    // 1 single empty list after visible styles
                    .append(new StyleSpan<>(Collections.emptyList(), lengthFollowingVisibleStyles));

            // no longer allow rich changes as setStyleSpans() will emit a rich change event,
            // which will lead to an infinite loop that will terminate with a StackOverflowError
            allowRichChange.suspendWhile(() ->
                    codeArea.setStyleSpans(0, styleSpans)
            );
        });

where getVisibleText() is:

public String getVisibleText() {
    return codeArea.getVisibleParagraphs().map(Paragraph::getText).reduce((a, b) -> a + "\n" + b).getOrElse("");
}

@Ansury
Copy link
Author

Ansury commented Jan 28, 2018

It seems a bit better doing the above, at least memory wise. Side effect I see here is that highlighting doesn't work during scrolling and takes a second to engage once scrolling stops. Not ideal but maybe okay. Still kinda sluggish when paging up/down. Another thing I noticed was this Error being thrown (so, maybe I have something screwed up), although it seems to run fine regardless.

Exception in thread "JavaFX Application Thread" java.lang.AssertionError: Unreachable code
at org.fxmisc.richtext.GenericStyledArea.visibleParToAllParIndex(GenericStyledArea.java:786)
at org.fxmisc.richtext.demo.ScalabilityTest.lambda$start$2(ScalabilityTest.java:145)
at org.reactfx.util.NonAccumulativeStreamNotifications.lambda$head$0(NotificationAccumulator.java:134)
at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:68)
at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:57)
at org.reactfx.ProperEventStream.emit(ProperEventStream.java:18)
at org.reactfx.util.NonAccumulativeStreamNotifications.lambda$head$0(NotificationAccumulator.java:134)
at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:68)
at org.reactfx.ObservableBase.notifyObservers(ObservableBase.java:57)
at org.reactfx.ProperEventStream.emit(ProperEventStream.java:18)
at org.reactfx.SuccessionReducingStream.handleTimeout(SuccessionReducingStream.java:73)
at org.reactfx.util.FxTimer.lambda$restart$0(FxTimer.java:89)
at com.sun.scenario.animation.shared.TimelineClipCore.visitKeyFrame(TimelineClipCore.java:239)
at com.sun.scenario.animation.shared.TimelineClipCore.playTo(TimelineClipCore.java:180)
at javafx.animation.Timeline.impl_playTo(Timeline.java:176)
...

ScalabilityTest.java:145:
int firstVisibleParIdx = codeArea.visibleParToAllParIndex(0);
That only showed up twice right at startup, so maybe just a symptom of how that demo initializes.

I did remove this bit from the original example above:

//        codeArea.richChanges()
//                .filter(ch -> !ch.getInserted().equals(ch.getRemoved())) // XXX
//                .subscribe(change -> {
//                    codeArea.setStyleSpans(0, computeHighlighting(getVisibleText(codeArea)));
//                });

I have to admit though, the functional reactive development style is painful for me. Either I'm getting old and stubborn or it's just unfamiliarity, but it really obfuscates tracking what the code is doing, for me. Between that and the fact that I really don't need highlighting (or even editing), maybe I'll just skip it.

@JordanMartinez
Copy link
Contributor

Side effect I see here is that highlighting doesn't work during scrolling and takes a second to engage once scrolling stops. Not ideal but maybe okay.

You could adjust the viewport delay (currently 200 ms in the code) to be less or remove it altogether and that might make things better.

Another thing I noticed was this Error being thrown (so, maybe I have something screwed up), although it seems to run fine regardless.
int firstVisibleParIdx = codeArea.visibleParToAllParIndex(0);
That only showed up twice right at startup, so maybe just a symptom of how that demo initializes.

This probably throws an error because I assumed (when writing the code) that the area is displayed on one's screen before that method ever gets called. So, it does sound like an initialization bug that could be resolved by adding a check like

EventStreams.merge(plainTextChange, dirtyViewport)
        .hook(c -> System.out.println("either the viewport has changed or a plain text change has occurred. fire an event"))
        // only run when the area is visible since it will always have
        // at least 1 paragraph that is visible
        .conditionOn(area.visibleProperty())
        .subscribe(dirtyStyles -> { /* rest of the code above */

I have to admit though, the functional reactive development style is painful for me. Either I'm getting old and stubborn or it's just unfamiliarity, but it really obfuscates tracking what the code is doing, for me. Between that and the fact that I really don't need highlighting (or even editing), maybe I'll just skip it.

It's probably just unfamiliarity with the style coupled with the frustration that something as simple as this shouldn't take up so much memory (like when compared with your basic already-installed text editors on Windows/Mac/Linux that respond very quickly to such changes) all adding up and compounding one another.

@JonathanMarchand
Copy link
Contributor

Hi,

I also noticed incredible amounts of RAM being used when I added a lot of lines.

After some investigation, I found out there's a memory leak. Each ParagraphText adds various listeners to the Caret, and those listeners directly reference the ParagraphText that adds them.
By adding lines to the ParagraphText using GenericStyledArea.replace(), the previous ParagraphText is supposed to be discarded, but it cannot be Garbage Collected as it is still attached to the Caret.

I've been working on a fix using WeakListeners that I'll be pushing shortly.

@Jugen Jugen closed this as completed in #779 Nov 4, 2018
Jugen added a commit that referenced this issue Nov 4, 2018
…-leak-in-paragraphtext

Issue #627 Extreme memory usage with large blocks of text
JFormDesigner pushed a commit to JFormDesigner/RichTextFX that referenced this issue Dec 15, 2018
…ret on dispose instead of waiting for GC (this improves PR FXMisc#779, issue FXMisc#627)

without this change, disposed ParagraphText objects still listen to selection and caret and do some useless stuff
Jugen pushed a commit that referenced this issue Dec 16, 2018
…s on disposal (#791)

* ParagraphText: immediately remove listeners from SelectionPath and Caret on dispose instead of waiting for GC (this improves PR #779, issue #627)

* ParagraphText: since weak listeners are no longer required for selection and caret listeners, remove listener classes again and move listener code to constructor (saves 100 lines of code)

* ParagraphText: MapChangeListener.Change.wasAdded() and wasRemoved() may both return true when replacing an item. So check both and handle wasRemoved() before wasAdded().

* Fixed (rare) NPE in class ParagraphText when GC frees object between two WeakReference.get() invocations
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

4 participants