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

Startup performance improvements #361

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Eskaan
Copy link

@Eskaan Eskaan commented Jan 22, 2022

How and how much time can we save

I followed your timing Todos and commented code, found that these two lines of code take about two seconds to run on the first call.
https://github.com/processing/processing4/blob/5fef895e84c20f4d30ed65a426e37c3c7397e5c7/app/src/processing/app/ui/Toolkit.java#L1225-L1226 By putting the font loading into a thread I was able to reduce the startup time by about two seconds.

I don't think this will cause bugs because

  1. According to my timings it will be done before the GUI loads anyways
  2. It is not responsible for the GUI font itself but only for the Font Setting - Even if it does fail it will be fine after reopeing the settings panel.

Difference in timings

Reports are seperated by three hyphens (---)
Notice the first font call (Mirroring in ManagerFrame->ContribInit->All)

Old New
image image

I want to test that

I also comited and pushed my timing code extending your code on this branch: codeac-timings
Merge the codeac-startup-performance branch into it to see the difference in the first Font call.

by moving the font registering code into its foreign Thread saving ~2seconds.
@Eskaan
Copy link
Author

Eskaan commented Jan 22, 2022

I initially followed your TODO comments but as it seems, the removal of the loop fixed it. Got up to only about a second for it by downloading every single library there is. I see this as no longer an issue (you can remove the todos).
EDIT: I mean the Todos leading to here:
https://github.com/processing/processing4/blob/5fef895e84c20f4d30ed65a426e37c3c7397e5c7/app/src/processing/app/contrib/ContributionListing.java#L332-L339

@Eskaan
Copy link
Author

Eskaan commented Jan 22, 2022

Also a question that is not related to the pr:
Would you be interested in an update of the IntelliJ project files? Had to fiddle around for a while to get it all working. Otherwise I will just add them to my .gitignore.

@benfry
Copy link
Owner

benfry commented Jan 22, 2022

Thanks for looking into it.

Please first check whether it's ge.registerFont(font) or GraphicsEnvironment.getLocalGraphicsEnvironment() that's taking all the time. It may be the latter if it's the first time that AWT classes are being loaded by the JVM, which means the 2 seconds will just happen elsewhere.

Also, most functions like this need to happen on the EDT, where it's safe to perform AWT operations. So I'm a little nervous that we can't just throw it in a non-EDT thread like this, but should make sure we're safe.

And one more, that font needs to be available for the rest of the UI. So we need to make sure it's loaded before anything else is in use, so if we make this change we're gonna have to ensure that none of the other UI elements that expect the font to be available are being initialized in the meantime.

@Eskaan
Copy link
Author

Eskaan commented Jan 22, 2022

It is not the class loading that takes that long, but the registering process.

I don't really know about AWT and EDT in Java - What should be unsave when calling this code in a separate thread? Also this code is first executed from the Main Thread (constructor of Base) (is this isn't the EDT Thread right?) so I think it should be possible to export the function into its own thread.
I will research all uses of GraphicsEnvironment.getAllFonts() will tell you in a moment.

I thought you were only using the registered font when calling for all accessible fonts in the settings, else using this function to get it (because it returns the font).

@Eskaan
Copy link
Author

Eskaan commented Jan 22, 2022

So the first use case is in the CreateFont class, which is unused beside SampleComponent which is completely unused.
The second usage is in the core module. Class PFont uses it in the loadFonts method whit two usages in public methods. One in list, which has no usages, and one called findFont used in the PGraphics class in it's createFont method. There the method is surrounded by a try-catch.
The third use is in the Toolkit class itself, in the getMonoFontList method. It is passed through another series of methods until it ends up in the PreferenceFrame class. We know where it is used there.

EDIT: All Editor functions refer to the getSansFont method in the Toolkit class, which is the method I edited.

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

Successfully merging this pull request may close these issues.

None yet

2 participants