PERF: flush topic timings less frequently

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)

diff --git a/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6
index db395dd..a1f101a 100644
--- a/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6
+++ b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6
@@ -236,15 +236,21 @@ export default MountWidget.extend({
     }
 
     const onscreenPostNumbers = [];
+    const readPostNumbers = [];
+
     const prev = this._previouslyNearby;
     const newPrev = {};
     nearby.forEach(idx => {
       const post = posts.objectAt(idx);
       const postNumber = post.post_number;
+
       delete prev[postNumber];
 
       if (onscreen.indexOf(idx) !== -1) {
         onscreenPostNumbers.push(postNumber);
+        if (post.read) {
+          readPostNumbers.push(postNumber);
+        }
       }
       newPrev[postNumber] = post;
       uncloak(post, this);
@@ -253,7 +259,7 @@ export default MountWidget.extend({
     Object.values(prev).forEach(node => cloak(node, this));
 
     this._previouslyNearby = newPrev;
-    this.screenTrack.setOnscreen(onscreenPostNumbers);
+    this.screenTrack.setOnscreen(onscreenPostNumbers, readPostNumbers);
   },
 
   _scrollTriggered() {
diff --git a/app/assets/javascripts/discourse/lib/screen-track.js.es6 b/app/assets/javascripts/discourse/lib/screen-track.js.es6
index 58982a1..3050de3 100644
--- a/app/assets/javascripts/discourse/lib/screen-track.js.es6
+++ b/app/assets/javascripts/discourse/lib/screen-track.js.es6
@@ -54,8 +54,9 @@ export default class {
     }
   }
 
-  setOnscreen(onscreen) {
+  setOnscreen(onscreen, readOnscreen) {
     this._onscreen = onscreen;
+    this._readOnscreen = readOnscreen;
   }
 
   // Reset our timers
@@ -68,6 +69,8 @@ export default class {
     this._totalTimings = {};
     this._topicTime = 0;
     this._onscreen = [];
+    this._readOnscreen = [];
+    this._readPosts = {};
     this._inProgress = false;
   }
 
@@ -112,6 +115,7 @@ export default class {
     if (!$.isEmptyObject(newTimings)) {
       if (this.currentUser) {
         this._inProgress = true;
+
         ajax("/topics/timings", {
           data: {
             timings: newTimings,
@@ -198,20 +202,27 @@ export default class {
     const nextFlush = this.siteSettings.flush_timings_secs * 1000;
 
     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)) {
       this.flush();
     }
 
-    // Don't track timings if we're not in focus
-    if (!Discourse.get("hasFocus")) return;
+    if (Discourse.get("hasFocus")) {
+      this._topicTime += diff;
 
-    this._topicTime += diff;
+      this._onscreen.forEach(
+        postNumber => (timings[postNumber] = (timings[postNumber] || 0) + diff)
+      );
 
-    this._onscreen.forEach(
-      postNumber => (timings[postNumber] = (timings[postNumber] || 0) + diff)
-    );
+      this._readOnscreen.forEach(postNumber => {
+        this._readPosts[postNumber] = true;
+      });
+    }
   }
 }
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 86ae8b3..ac714ca 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1443,7 +1443,7 @@ developer:
     max: 99000
   flush_timings_secs:
     client: true
-    default: 20
+    default: 60
   active_user_rate_limit_secs: 60
   verbose_localization:
     default: false
diff --git a/test/javascripts/lib/screen-track-test.js.es6 b/test/javascripts/lib/screen-track-test.js.es6
new file mode 100644
index 0000000..b356dc9
--- /dev/null
+++ b/test/javascripts/lib/screen-track-test.js.es6
@@ -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);
+});

GitHub sha: 7068b58e

2 Likes