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

Performance Profile #411

Closed
ghost opened this issue Dec 29, 2016 · 5 comments
Closed

Performance Profile #411

ghost opened this issue Dec 29, 2016 · 5 comments

Comments

@ghost
Copy link

ghost commented Dec 29, 2016

Profile information from my application:

  • 4.3% + 1.5% of time is spent in com.sun.javafx.collections.VetoableListDecorator.add, which is called from org.fxmisc.richtext.ParagraphText.<init>.

ParagraphText

The following objects are instantiated for each paragraph, regardless of whether they get used:

    private final Path caretShape = new Path();
    private final Path selectionShape = new Path();
    private final List<Path> backgroundShapes = new ArrayList<>();
private final List<Path> underlineShapes = new ArrayList<>();

The selection shape, in particular, probably doesn't need to have an object assigned until needed. This implies that the following lines in the constructor might also be suitable for lazy initialization:

        // selection highlight
        selectionShape.setManaged(false);
        selectionShape.setFill(Color.DODGERBLUE);
        selectionShape.setStrokeWidth(0);
        selectionShape.layoutXProperty().bind(leftInset);
        selectionShape.layoutYProperty().bind(topInset);
getChildren().add(selectionShape);

TextExt

Another minor hot-spot in the application came from lines 113 to 125 of TextExt. If it is possible to cache the superclass' CSS meta data, rather than create a new list each time, it might offer a small performance boost.

@TomasMikula
Copy link
Member

TomasMikula commented Mar 29, 2017

Thanks, this is useful information. My guess would be that selectionShape and caretShape are not such a big deal, since they are just one of each per paragraph. OTOH, backgroundShapes and underlineShapes are instantiated one of each per styled segment, even when they are not actually used (which is most of the time). I was aware of this when implementing background color, but didn't have an idea of what its cost was.

@JordanMartinez
Copy link
Contributor

JordanMartinez commented Mar 29, 2017

Another minor hot-spot in the application came from lines 113 to 125 of TextExt. If it is possible to cache the superclass' CSS meta data, rather than create a new list each time, it might offer a small performance boost.

Well, that can be improved easily. The same could be done on GenericStyledArea as it doesn't cache it either... @DaveJarvis Why not submit a PR for that?

@ghost
Copy link
Author

ghost commented Mar 30, 2017

SegmentOps doesn't compile in NetBeans against master:

    @Override
    public Optional<Either<L, R>> join(Either<L, R> left, Either<L, R> right) {
        return left.unify(ll -> right.unify(rl -> lOps.join(ll, rl).map(Either::left), rr -> Optional.empty()),
                          lr -> right.unify(rl -> Optional.empty(), rr -> rOps.join(lr, rr).map(Either::right)));
    }

The return statement does not evaluate to an Optional<Either<L, R>>, according to the IDE, as per its interface definition:

    public Optional<SEG> join(SEG currentSeg, SEG nextSeg);

FWIW, aligning symmetrical code can sometimes make spotting issues easier, such as:

    @Override
    public Optional<Either<L, R>> join(Either<L, R> left, Either<L, R> right) {
        return left.unify(
          ll -> right.unify(
            rl -> lOps.join(ll, rl).map(Either::left),
            rr -> Optional.empty()
          ),
          lr -> right.unify(
            rl -> Optional.empty(), 
            rr -> rOps.join(lr, rr).map(Either::right)
          )
        );
    }

The code, without knowing what it's supposed to do, looks correct, except for the left.unify statement not returning the data type expected by NetBeans. I'm running Java 1.8 121.

@JordanMartinez
Copy link
Contributor

The above code was implemented by @afester.
My IDE (IntelliJ) reports no issue with that code and gradle build works without issue.

@JordanMartinez
Copy link
Contributor

I'm closing this as the issues raised are now indicated in separate issues.

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

2 participants