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

Fix startup speed #2747

Merged
merged 3 commits into from Feb 7, 2021
Merged

Fix startup speed #2747

merged 3 commits into from Feb 7, 2021

Conversation

b4n
Copy link
Member

@b4n b4n commented Feb 6, 2021

Dynamic margin width computation introduced in 1.37 is "slow", and can lead to about 3× slowdowns, which is most noticeable when loading startup files.

Fixes #2649. Probably fixes geany/geany-osx#20 as well.

This PR is mostly FTR and to let @techee check on OSX, there has already been some review and testing (see commit comments and comments starting at #2649 (comment)). But more reviews & tests are welcome of course.

Avoid recomputing margin widths 2 or 3 times when initially creating
the editing widget.  As computing the margin widths might be costly,
this can make widget creation about twice as fast.

Part of #2649.
Computing the line height is a very costly operation that involves font
loading and measuring, but the value stays mostly the same over time,
as it depends on font, zoom and a couple other style settings which
rarely change.

As line height used to compute margin widths dominates startup timings,
we now cache the latest result.  This caching makes line height
computation barely noticeable in startup times now.

Fixes #2649.
It voids any kind of clashes, and is possibly easier to understand.
It results in a slight memory overhead sorting a little more cache
data, but it should not matter much here.
@b4n b4n self-assigned this Feb 6, 2021
@b4n b4n merged commit e027e24 into master Feb 7, 2021
@b4n
Copy link
Member Author

b4n commented Feb 7, 2021

If this proves not to cause issues (it really shouldn't), it's possibly candidate for a 1.37.2. Opinions? @elextr @eht16 @codebrainz

@b4n b4n added this to the 1.38 milestone Feb 7, 2021
@eht16
Copy link
Member

eht16 commented Feb 7, 2021

I think releasing it is a good idea. Basically, I wouldn't mind whether we call it 1.37.2 or 1.38 except that just "ctags_sync" has landed. So yes, 1.37.2 seems sensible.
However, I personally won't be able to do the Windows builds until approx. mid of March.

@b4n b4n modified the milestones: 1.38, 1.37.2 Feb 7, 2021
@elextr
Copy link
Member

elextr commented Feb 7, 2021

Given the "only" other change is we have a new ctags and it sounds like mid march before release and we havn't broken GTK2 (yet) I'd say call it 1.37.2.

@b4n
Copy link
Member Author

b4n commented Feb 8, 2021

Given the "only" other change is we have a new ctags

I don't think I'm gonna put the new ctags in a point release; although it's fairly minor from a user perspective, I'm not very comfortable pretending a +30k -7k change still is 1.37.

@elextr
Copy link
Member

elextr commented Feb 8, 2021

I'm not very comfortable pretending a +30k -7k change still is 1.37.

Well branch the 1.37.2 off the commit before ctags maybe.

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.

Slow startup Long startup times after upgrade to 1.37
4 participants