DEV: Topic tracking state improvements (#13218)
I merged this PR in yesterday, finally thinking this was done DEV: Topic tracking state improvements by martin-brennan · Pull Request #12958 · discourse/discourse · GitHub but then a wild performance regression occurred. These are the problem methods:
Turns out date comparison is super expensive on the backend as well as the frontend.
The fix was to just move the
treat_as_new_topic_start_date into the SQL query rather than using the slower
UserOption#treat_as_new_topic_start_date method in ruby. After this change, 1% of the total time is spent with the
created_in_new_period comparison instead of ~20%.
Original PR which had to be reverted https://github.com/discourse/discourse/pull/12555. See the description there for what this PR is achieving, plus below.
The issue with the original PR is addressed in DEV: Topic tracking state improvements by martin-brennan · Pull Request #12958 · discourse/discourse · GitHub
If you went to the
x unread link for a tag Chrome would freeze up and possibly crash, or eventually unfreeze after nearly 10 mins. Other routes for unread/new were similarly slow. From profiling the issue was the
sync function of
topic-tracking-state.js, which calls down to
isNew which in turn calls
moment, a change I had made in the PR above. The time it takes locally with ~1400 topics in the tracking state is 2.3 seconds.
To solve this issue, I have moved these calculations for “created in new period” and “unread not too old” into the tracking state serializer.
When I was looking at the profiler I also noticed this issue which was just compounding the problem. Every time we modify topic tracking state we recalculate the sidebar tracking/everything/tag counts. However this calls
countTags which can be quite expensive as they go through the whole tracking state (and were also calling the removed moment functions).
I added some logs and this was being called 30 times when navigating to a new /unread route because
sync is being called from
build-topic-route (one for each topic loaded due to pagination). So I just added a debounce here and it makes things even faster.
Finally, I changed topic tracking state to use a Map so our counts of the state keys is faster (Maps have .size whereas objects you have to do Object.keys(obj) which is O(n).)
GitHub sha: e15c86e8