e15c86e8c5
I merged this PR in yesterday, finally thinking this was done https://github.com/discourse/discourse/pull/12958 but then a wild performance regression occurred. These are the problem methods:1aa20bd681/app/serializers/topic_tracking_state_serializer.rb (L13-L21)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%. ---- History: 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 in92ef54f402If 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 `forEachTracked` and `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).) <!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
69 lines
2.0 KiB
JavaScript
69 lines
2.0 KiB
JavaScript
import { observes, on } from "discourse-common/utils/decorators";
|
|
import { schedule, scheduleOnce } from "@ember/runloop";
|
|
import Component from "@ember/component";
|
|
import LoadMore from "discourse/mixins/load-more";
|
|
import UrlRefresh from "discourse/mixins/url-refresh";
|
|
import { inject as service } from "@ember/service";
|
|
|
|
const DiscoveryTopicsListComponent = Component.extend(UrlRefresh, LoadMore, {
|
|
classNames: ["contents"],
|
|
eyelineSelector: ".topic-list-item",
|
|
documentTitle: service(),
|
|
|
|
@on("didInsertElement")
|
|
@observes("model")
|
|
_readjustScrollPosition() {
|
|
const scrollTo = this.session.get("topicListScrollPosition");
|
|
if (scrollTo && scrollTo >= 0) {
|
|
schedule("afterRender", () => $(window).scrollTop(scrollTo + 1));
|
|
} else {
|
|
scheduleOnce("afterRender", this, this.loadMoreUnlessFull);
|
|
}
|
|
},
|
|
|
|
@on("didInsertElement")
|
|
_monitorTrackingState() {
|
|
this.topicTrackingState.onStateChange(() => this._updateTrackingTopics());
|
|
},
|
|
|
|
@on("willDestroyElement")
|
|
_removeTrackingStateChangeMonitor() {
|
|
this.topicTrackingState.offStateChange(this.stateChangeCallbackId);
|
|
},
|
|
|
|
_updateTrackingTopics() {
|
|
this.topicTrackingState.updateTopics(this.model.topics);
|
|
},
|
|
|
|
@observes("incomingCount")
|
|
_updateTitle() {
|
|
this.documentTitle.updateContextCount(this.incomingCount);
|
|
},
|
|
|
|
saveScrollPosition() {
|
|
this.session.set("topicListScrollPosition", $(window).scrollTop());
|
|
},
|
|
|
|
actions: {
|
|
loadMore() {
|
|
this.documentTitle.updateContextCount(0);
|
|
this.model.loadMore().then(({ moreTopicsUrl, newTopics } = {}) => {
|
|
if (
|
|
newTopics &&
|
|
newTopics.length &&
|
|
this.autoAddTopicsToBulkSelect &&
|
|
this.bulkSelectEnabled
|
|
) {
|
|
this.addTopicsToBulkSelect(newTopics);
|
|
}
|
|
schedule("afterRender", () => this.saveScrollPosition());
|
|
if (moreTopicsUrl && $(window).height() >= $(document).height()) {
|
|
this.send("loadMore");
|
|
}
|
|
});
|
|
},
|
|
},
|
|
});
|
|
|
|
export default DiscoveryTopicsListComponent;
|