PERF: flush topic timings less frequently

This is a first step of a performance optimisation, more will follow

Previously we did not properly account for previously read topics while
"rushing" marking times on posts.

The new mechanism now avoids "rushing" sending timings to server if all
the posts were read.

Also to alleviate some server load we only "ping" the server with old timings
once a minute (it used to be every 20 seconds)
This commit is contained in:
Sam Saffron
2019-04-18 17:30:39 +10:00
parent 1f17d52f55
commit 7068b58e85
4 changed files with 102 additions and 10 deletions

View File

@ -236,15 +236,21 @@ export default MountWidget.extend({
} }
const onscreenPostNumbers = []; const onscreenPostNumbers = [];
const readPostNumbers = [];
const prev = this._previouslyNearby; const prev = this._previouslyNearby;
const newPrev = {}; const newPrev = {};
nearby.forEach(idx => { nearby.forEach(idx => {
const post = posts.objectAt(idx); const post = posts.objectAt(idx);
const postNumber = post.post_number; const postNumber = post.post_number;
delete prev[postNumber]; delete prev[postNumber];
if (onscreen.indexOf(idx) !== -1) { if (onscreen.indexOf(idx) !== -1) {
onscreenPostNumbers.push(postNumber); onscreenPostNumbers.push(postNumber);
if (post.read) {
readPostNumbers.push(postNumber);
}
} }
newPrev[postNumber] = post; newPrev[postNumber] = post;
uncloak(post, this); uncloak(post, this);
@ -253,7 +259,7 @@ export default MountWidget.extend({
Object.values(prev).forEach(node => cloak(node, this)); Object.values(prev).forEach(node => cloak(node, this));
this._previouslyNearby = newPrev; this._previouslyNearby = newPrev;
this.screenTrack.setOnscreen(onscreenPostNumbers); this.screenTrack.setOnscreen(onscreenPostNumbers, readPostNumbers);
}, },
_scrollTriggered() { _scrollTriggered() {

View File

@ -54,8 +54,9 @@ export default class {
} }
} }
setOnscreen(onscreen) { setOnscreen(onscreen, readOnscreen) {
this._onscreen = onscreen; this._onscreen = onscreen;
this._readOnscreen = readOnscreen;
} }
// Reset our timers // Reset our timers
@ -68,6 +69,8 @@ export default class {
this._totalTimings = {}; this._totalTimings = {};
this._topicTime = 0; this._topicTime = 0;
this._onscreen = []; this._onscreen = [];
this._readOnscreen = [];
this._readPosts = {};
this._inProgress = false; this._inProgress = false;
} }
@ -112,6 +115,7 @@ export default class {
if (!$.isEmptyObject(newTimings)) { if (!$.isEmptyObject(newTimings)) {
if (this.currentUser) { if (this.currentUser) {
this._inProgress = true; this._inProgress = true;
ajax("/topics/timings", { ajax("/topics/timings", {
data: { data: {
timings: newTimings, timings: newTimings,
@ -198,20 +202,27 @@ export default class {
const nextFlush = this.siteSettings.flush_timings_secs * 1000; const nextFlush = this.siteSettings.flush_timings_secs * 1000;
const rush = Object.keys(timings).some(postNumber => { const rush = Object.keys(timings).some(postNumber => {
return timings[postNumber] > 0 && !totalTimings[postNumber]; return (
timings[postNumber] > 0 &&
!totalTimings[postNumber] &&
!this._readPosts[postNumber]
);
}); });
if (!this._inProgress && (this._lastFlush > nextFlush || rush)) { if (!this._inProgress && (this._lastFlush > nextFlush || rush)) {
this.flush(); this.flush();
} }
// Don't track timings if we're not in focus if (Discourse.get("hasFocus")) {
if (!Discourse.get("hasFocus")) return; this._topicTime += diff;
this._topicTime += diff; this._onscreen.forEach(
postNumber => (timings[postNumber] = (timings[postNumber] || 0) + diff)
);
this._onscreen.forEach( this._readOnscreen.forEach(postNumber => {
postNumber => (timings[postNumber] = (timings[postNumber] || 0) + diff) this._readPosts[postNumber] = true;
); });
}
} }
} }

View File

@ -1443,7 +1443,7 @@ developer:
max: 99000 max: 99000
flush_timings_secs: flush_timings_secs:
client: true client: true
default: 20 default: 60
active_user_rate_limit_secs: 60 active_user_rate_limit_secs: 60
verbose_localization: verbose_localization:
default: false default: false

View File

@ -0,0 +1,75 @@
import TopicTrackingState from "discourse/models/topic-tracking-state";
import Session from "discourse/models/session";
import ScreenTrack from "discourse/lib/screen-track";
let clock;
QUnit.module("lib:screen-track", {
beforeEach() {
clock = sinon.useFakeTimers(new Date(2012, 11, 31, 12, 0).getTime());
},
afterEach() {
clock.restore();
}
});
QUnit.test("Correctly flushes posts as needed", assert => {
const timings = [];
// prettier-ignore
server.post("/topics/timings", t => { //eslint-disable-line
timings.push(t);
return [200, {}, ""];
});
const trackingState = TopicTrackingState.create();
const siteSettings = {
flush_timings_secs: 60
};
const currentUser = { id: 1, username: "sam" };
const tracker = new ScreenTrack(
trackingState,
siteSettings,
Session.current(),
currentUser
);
const topicController = null;
Discourse.set("hasFocus", true);
tracker.reset();
tracker.start(1, topicController);
tracker.setOnscreen([1, 2, 3], [1, 2, 3]);
clock.tick(1050);
clock.tick(1050);
// no ajax yet
assert.equal(0, timings.length);
tracker.setOnscreen([1, 2, 3, 4], [1, 2, 3]);
clock.tick(1050);
clock.tick(1050);
// we should be rushed now cause there is a new thing on the screen
assert.equal(1, timings.length);
const req =
"timings%5B1%5D=3000&timings%5B2%5D=3000&timings%5B3%5D=3000&timings%5B4%5D=1000&topic_time=3000&topic_id=1";
assert.equal(timings[0].requestBody, req);
tracker.stop();
assert.equal(2, timings.length);
const req2 =
"timings%5B1%5D=1200&timings%5B2%5D=1200&timings%5B3%5D=1200&timings%5B4%5D=1200&topic_time=1200&topic_id=1";
assert.equal(timings[1].requestBody, req2);
});