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

Rework throttled refresh #8677

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

BurningTreeC
Copy link
Contributor

This PR reworks the throttled refresh so that it works for the styleRefresh, too.
It adds two switches so that we can control if we want only the styleRefresh to throttle or only the mainRefresh to throttle.
It also makes the styleRefresh in new Windows equal to the styleRefresh in the main window.
Also in new windows the throttling is added, the same way as in the main window.

Copy link

Confirmed: BurningTreeC has already signed the Contributor License Agreement (see contributing.md)

@BurningTreeC BurningTreeC marked this pull request as ready for review October 14, 2024 05:14
@BurningTreeC BurningTreeC marked this pull request as draft October 15, 2024 16:18
@BurningTreeC BurningTreeC marked this pull request as ready for review October 15, 2024 19:45
@BurningTreeC
Copy link
Contributor Author

This now works and deferres the changes for both styleRefresh and mainRefresh in both the main window and new windows

@BurningTreeC
Copy link
Contributor Author

@Jermolene @pmario @saqimtiaz here's a build with performance instrumentation enabled:

https://rework-throttled-refresh.tiddlyhost.com/

@pmario
Copy link
Member

pmario commented Oct 16, 2024

... here's a build with performance instrumentation enabled:

Seems to be slightly faster than the tiddlywiki/prerelease

@Leilei332
Copy link
Contributor

Actually I wonder why opening $:/TagManager and $:/Manager is especially slow when there are many tiddlers or tags.

@BurningTreeC
Copy link
Contributor Author

Actually I wonder why opening $:/TagManager and $:/Manager is especially slow when there are many tiddlers or tags.

Hello @Leilei332 - do you notice this in general or in this PR?

@Leilei332
Copy link
Contributor

Actually I wonder why opening $:/TagManager and $:/Manager is especially slow when there are many tiddlers or tags.

Hello @Leilei332 - do you notice this in general or in this PR?

This is a general problem in tiddlywiki. In this PR it is a little quicker.

Tag Manager (PR):

performance: styleRefresh: 8.00ms
performance: mainRefresh: 1103.00ms

Tag Manager (prerelease):

performance: styleRefresh: 13.00ms
performance: mainRefresh: 1122.00ms

Manager (PR):

performance: styleRefresh: 17.00ms
performance: mainRefresh: 513.00ms

Manager (prerelease):

performance: styleRefresh: 16.00ms
performance: mainRefresh: 514.00ms

@Leilei332
Copy link
Contributor

Leilei332 commented Oct 18, 2024

I think the two tiddlers should be improved because its performance is slow on mobile phones. The result is the same (performance in this PR is a little faster than prerelease)

This PR:

IMG_20241018_172021.jpg

Prerelease:

IMG_20241018_172124.jpg

@Jermolene
Copy link
Member

Thanks @BurningTreeC this is promising, but I don't think we're ready to include it in v5.3.6.

One issue is that the code of render.js is now pretty gnarly and hard to follow. I think we need to consider refactoring the rendering and refreshing into a generic class that can be used both for styles and body content. In a separate branch I've been exploring the idea with a new "wikifier" class. See this commit. It will need to be extended to allow it to replace all the occurrences of rendering/refreshing in the system.

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.

4 participants