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

[BUG] Compare Tiddlywiki 5.1.23pre and 5.1.22 for adding Tags #5188

Closed
kookma opened this issue Dec 3, 2020 · 45 comments
Closed

[BUG] Compare Tiddlywiki 5.1.23pre and 5.1.22 for adding Tags #5188

kookma opened this issue Dec 3, 2020 · 45 comments

Comments

@kookma
Copy link
Contributor

kookma commented Dec 3, 2020

Describe the bug
It seems TW 5.1.23 is slower in adding tags compared to TW 5.1.22. See below screen recording, the measured time is 4 seconds more for TW 5.123 (15 vs. 19).

Screen Recordings
TW5 1 22
TW5 1 23

Is there any way to speedup the process in TW5.1.23? This lag is considerable when you have alot of tags.

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser Chrome 87
  • Version 5.122 and 5.123
@BurningTreeC
Copy link
Contributor

@kookma this lag depends on system performance and yes, it's more obvious when there are many tags. The problem is the tagpicker setting focus on the input field right after adding a tag, which makes the tag-dropdown reload the full tags-list

@Jermolene
Copy link
Owner

Thanks @kookma @BurningTreeC this seems like it could impact a lot of users. I wonder if there's any potential mitigations?

My first thought is probably too complicated: that in the case that the new tag text input receives focus via the keyboard that we don't display the dropdown until the user types something, or clicks on the arrow.

I'll do some performance analysis when I get back to my desk.

@BurningTreeC
Copy link
Contributor

@Jermolene - my first thought was making it configurable if the tags-input should get focus right after adding a tag

@BurningTreeC
Copy link
Contributor

@kookma @Jermolene - there's still the $:/config/Tags/MinLength setting that would help here

@Jermolene
Copy link
Owner

One consideration is that displaying the dropdown on focus is very useful when using the mouse.

@BurningTreeC
Copy link
Contributor

One consideration is that displaying the dropdown on focus is very useful when using the mouse.

Yes, indeed.

@BurningTreeC
Copy link
Contributor

@kookma @Jermolene - I wonder if retain="yes" on the tags-dropdown would speed things up a little

@BurningTreeC
Copy link
Contributor

@kookma - could you try changing the line 73 (in $:/core/macros/tag-picker):

from:

<$reveal state=<<qualify "$:/state/popup/tags-auto-complete">> type="nomatch" text="" default="">

to:

<$reveal state=<<qualify "$:/state/popup/tags-auto-complete">> type="nomatch" text="" default="" retain="yes">

and see if that has a performance impact in your tests?

@BurningTreeC
Copy link
Contributor

An idea would be a setting that shows the tags in the tags drop down as simple text if set to yes and show the tag pills in all other cases. That has a significant performance impact

@kookma
Copy link
Contributor Author

kookma commented Dec 3, 2020

@BurningTreeC, @Jermolene
I was not at my desk, I will give a try using your suggestion and back to you

@kookma
Copy link
Contributor Author

kookma commented Dec 3, 2020

@kookma - could you try changing the line 73 (in $:/core/macros/tag-picker):

Tested, it has not sensible effects. It seems the step tag is added (after selection) is the slowest!
Also the for the first tag it takes more to display drop down menu

See screen recording (time after 18.87s before 19sec)

TW5123_retain_yes

@kookma
Copy link
Contributor Author

kookma commented Dec 3, 2020

@kookma @Jermolene - there's still the $:/config/Tags/MinLength setting that would help here

I set the $:/config/Tags/MinLength to 3. So it wont show the tags with one or two chars but the difference is considerable!!! before: 19sec, after:12 sec

TW5123_minlength_3

Note that there is no drop down before entering 3 chars, this will cutdown the number of tags to be displayed.

@Jermolene
Copy link
Owner

Hi @BurningTreeC I did some quick profiling with Chrome of repeatedly displaying and hiding the tag dropdown in tw5.com. It seems that there's quite a lot of parsing overhead. I would expect that to be because the tag rendering is making a lot of use of macros, rather than transcluding separate tiddlers. Macros have to be parsed afresh each time they are needed, and the results of parsing can't be cached like they can for parsing tiddlers.

So, I suspect that the best improvement might be if we can make a template that renders simple, non-draggable tag pills without rendering macros, just one or more templates that exist as separate tiddlers. The implication is that we'd have to use variables to pass the parameters.

@BurningTreeC
Copy link
Contributor

@Jermolene I think that's doable

@BurningTreeC
Copy link
Contributor

@Jermolene, if I understand you right, you'd like to replace the tag-button macrocalls that call the tag-pill macro with a separate, transcluded tiddler, is that right? As far as my testing goes, the tag-pill-inner macrocall is needed, the others can be replaced

@BurningTreeC
Copy link
Contributor

P.S. I think my system is too fast to measure a performance change

@kookma
Copy link
Contributor Author

kookma commented Dec 3, 2020

@BurningTreeC the above tests were done on a Core-i7 with 8GB of RAM, but if you have higher RAM like 32GB, then the difference is considerable.

@BurningTreeC
Copy link
Contributor

@kookma - would you mind testing this file, if you have time?

tag_picker_speed.zip

@kookma
Copy link
Contributor Author

kookma commented Dec 3, 2020

@kookma - would you mind testing this file, if you have time?

tag_picker_speed.zip

@BurningTreeC - I tested the revised code and it seems it has better performance. In this test, it took around 14.25 sec which is comparable to TW 5.1.22 (repeated test for 5.1.22 is 12.47sec)

See screen recording
TW5 1 23_New

@BurningTreeC
Copy link
Contributor

Thanks for testing @kookma! @Jermolene performance profiling shows that with your suggested changes we are again at the same performance level like 5.1.22 - we gain half a second for each dropdown rendering in my tests

@BurningTreeC
Copy link
Contributor

P.S. I'm on a very performant Linux system so performance gains might be bigger on less performant systems

@Jermolene
Copy link
Owner

Thanks @BurningTreeC @kookma

@BurningTreeC
Copy link
Contributor

@kookma , could you test the current prerelease and see if this issue is solved for you?

@kookma
Copy link
Contributor Author

kookma commented Dec 4, 2020

@BurningTreeC Is this the latest to test?

Prerelease built from branch 'master' at commit ae5d78b of https://github.com/Jermolene/TiddlyWiki5 at 2020-12-04 19:16:21 UTC

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@BurningTreeC , @Jermolene Tested with Prerelease built from branch 'master' at commit ae5d78b of https://github.com/Jermolene/TiddlyWiki5 at 2020-12-04 19:16:21 UTC

I don't see any improvement comparing to #5188 (comment) even a little slower. This below test takes 13.4 sec. Of course all the reported times are what I see from recorder, but they are good for comparison purpose.

The slowest step

The slowest step is: when the tag is clicked to be added and TW want to jump to the next tag pill to show the dropdown.

TW5 1 23_latest

@saqimtiaz
Copy link
Contributor

@kookma I think what is in the pre-release now is the same code that yielded the performance improvement that you saw in #5188 (comment) and felt was comparable to 5.1.22.

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

Thank you @saqimtiaz for confirmation. Well yes, the two (TW 5.1.22 and 5.123) are almost the same, except the sensible lag (slowness) in 5.1.23 exactly after clicking a tag to be added.

@saqimtiaz
Copy link
Contributor

@kookma do you find the current performance in pre-release acceptable compared to 5.1.22 or still too slow?

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@saqimtiaz I think they are the same now, but a I said in my previous post, there is a sensible lag/slowness after clicking a tag and TW wants to add and go to the next tag pill!

@BurningTreeC
Copy link
Contributor

@kookma @saqimtiaz - maybe we should restore the behavior that when clicking the tag from the dropdown, the focus is not automatically set to the tags input field right after ... that would also remove the lag @kookma is talking about. When choosing a tag via the keyboard the focus would still be set to the tags input right after

@BurningTreeC
Copy link
Contributor

@kookma - how is the lag if you set the animation duration to 0?

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@BurningTreeC I can NOT see any difference between the two (0 animation duration and 400ms). But as I told above the $:/config/Tags/MinLength set to 3 has a huge effect! I even set this value to 1 and again I got sensible difference!
I think it clearly indicates that the processing is slow! Set $:/config/Tags/MinLength to 1 filters the list of tags to say all those start with one letter say a or any other letter.

My conclusion: The average time for adding three tags as described above is almost the same for Tiddlywiki 5.1.22 and 5.1.23. The lag between the two points: (1) adding the first tag and (2) displaying the drop down in the next slot (tag pill) for 5.1.23 is sensible.

@BurningTreeC
Copy link
Contributor

@kookma - I have a little bit different version here:

tag_picker_speed_2.zip

It may be that you notice no difference at all... but it could be worth a try.

An alternative would be a control-panel setting that allows showing the tags in the tag-dropdown as plain text. Here's a version that shows the tags as plain text:

tag_picker_speed_3.zip

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@BurningTreeC
NEW TESTS

tag_picker_speed_2.zip shows a lit bit faster than what I did on https://tiddlywiki.com/prerelease/
tag_picker_speed_3.zip around 3 seconds faster. so by now, tag_picker_speed_3.zip is the fastest.

@BurningTreeC
Copy link
Contributor

@kookma - what would you prefer? An option in the control-panel to show the tags like in tag_picker_speed_3.zip or tag_picker_speed_2.zip?

@saqimtiaz
Copy link
Contributor

I feel like a small performance difference is acceptable from 5.1.22, considering that in 5.1.23 the field is focused which saves time when adding multiple tags.

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@BurningTreeC I prefer tag_picker_speed_2.zip
The reason is most users have small Tiddlywiki for their notes I guess more than half and for small wikis, the current configuration is speedy in modern computers of today!

@BurningTreeC
Copy link
Contributor

@kookma @saqimtiaz @Jermolene tag_picker_speed_2.zip uses the wikify widget on each tag-pill but appears to have a reasonable speed because it uses one less macrocall (doesn't use tag-pill-inner). @kookma would you mind going sure that it really improves speed in comparison to the current prerelease?

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@BurningTreeC What I reported is

  1. Windows 10, Chrome 87, 8MB of Ram
  2. Open the Tiddlywiki (test cases) adding a new tiddler
  3. Trun on screen recorder and add three tags: Android, HelloThere, and done
  4. Use mouse click for adding tag
  5. After last tag is added stop the recorder and see the elapsed time

So this is what user senses how the whole process is fast or slow.
As I explained in all previous posts the lag is sensible between adding the tag and going to next pill

So, may be other can perform some more tests and then you can decide.

@BurningTreeC
Copy link
Contributor

Performance profiling with Chrome shows that tag_picker_speed_2.zip is indeed faster

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@BurningTreeC how fast is tag_picker_speed_2.zip? Would you mind to share the results?

@BurningTreeC
Copy link
Contributor

I have two test-wikies based on the current prerelease with over 5000 tags. In both I trigger the tags dropdown repeatedly:

tag_picker_speed_2.zip:

Bildschirmfoto vom 2020-12-05 18-00-40

current prerelease:

Bildschirmfoto vom 2020-12-05 18-02-32

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@BurningTreeC Thank you for sharing. Your numbers are more reliable that what I measured using screen recorder.

@BurningTreeC
Copy link
Contributor

@kookma, #5207 is merged which incorporates tag_picker_speed_2.zip. Is it ok for you to close this issue?

@kookma
Copy link
Contributor Author

kookma commented Dec 5, 2020

@BurningTreeC
Sure, Thank you for all your efforts!

@kookma kookma closed this as completed Dec 5, 2020
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