-
Notifications
You must be signed in to change notification settings - Fork 30
Draft: [Event Highlighting]: Add a web worker for outsourcing the marker calculation #223
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
base: master
Are you sure you want to change the base?
Changes from all commits
1c392c5
a501056
e6cd4b3
9a0c4a2
f2a275a
2d7c2c3
43d68f2
96ef2a4
7739556
bfe6785
4006795
ee0c4f0
903960c
7023d6a
6d71198
ee5cf41
82813bc
e8a03ff
36159a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| const EventMapHelper = require('./event-map-helper') | ||
|
|
||
| require("../polyfills/set"); | ||
|
|
||
| const FRAME_RATE = 1000/30; | ||
|
|
||
| let messageBuffer = new Map(); | ||
| let receivedThisFrame = new Map(); | ||
|
|
||
| self.onmessage = function(e) { | ||
| const event = e.data; | ||
|
|
||
| queueEvent(event); | ||
| }; | ||
|
|
||
| setInterval(() => { | ||
| const {active, added, removed} = EventMapHelper.diffEventMaps( | ||
| messageBuffer, | ||
| receivedThisFrame, | ||
| ); | ||
|
|
||
| postMessage({active, added, removed }); | ||
| messageBuffer = EventMapHelper.setToMap(added.union(active)); | ||
| receivedThisFrame.clear(); | ||
|
|
||
| }, FRAME_RATE); | ||
|
|
||
| /** Helper that creates (or returns existing) nested maps. */ | ||
| function ensureNestedMap(root, key) { | ||
| if (!root.has(key)) root.set(key, new Map()); | ||
| return root.get(key); | ||
| } | ||
|
|
||
| /** Store events until the next animation frame */ | ||
| function queueEvent(event) { | ||
| const recvMap = ensureNestedMap(receivedThisFrame, event.eventId); | ||
|
|
||
| if (!recvMap.has(event.colStart)) { | ||
| recvMap.set(event.colStart, event); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| 'use babel' | ||
|
|
||
| class EventMapHelper { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I admit, I'm not the biggest fan of "helpers" and "utils" classes, so I'd suggest to avoid them as much as possible. In this case, both the functions are only used by the worker, so could them be moved there?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Me neither. But I am not able to define functions within the web worker and test them properly, because the jasmine tests only works within the main thread. This means I can not simply export them, because a worker is not a module. This would be the tradeoff: extract them somehow (class method or static function) or removing the tests. And to test the worker within the main thread is already an integration test. |
||
|
|
||
| static diffEventMaps(prevEvents, currentEvents) { | ||
| const removed = new Set(); | ||
| const added = new Set(); | ||
| const active = new Set(); | ||
|
|
||
| for (const [event, prevCols] of prevEvents) { | ||
| const currCols = currentEvents.get(event); | ||
| if (!currCols) { | ||
| for (const [, prevEvt] of prevCols) removed.add(prevEvt); | ||
| continue; | ||
| } | ||
|
|
||
| for (const [col, prevEvt] of prevCols) { | ||
| if (!currCols.has(col)) { | ||
| removed.add(prevEvt); | ||
| } else { | ||
| active.add(prevEvt); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for (const [event, currCols] of currentEvents) { | ||
| const prevCols = prevEvents.get(event); | ||
| if (!prevCols) { | ||
| for (const [, currEvt] of currCols) added.add(currEvt); | ||
| continue; | ||
| } | ||
|
|
||
| for (const [col, currEvt] of currCols) { | ||
| if (!prevCols.has(col)) { | ||
| added.add(currEvt); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return { removed, added, active}; | ||
| } | ||
|
|
||
| static setToMap(events) { | ||
| const resultEvents = new Map(); | ||
| events.forEach(event => { | ||
| if (!resultEvents.get(event.eventId)) { | ||
| resultEvents.set(event.eventId, new Map()); | ||
| } | ||
| const cols = resultEvents.get(event.eventId); | ||
|
|
||
| cols.set(event.colStart, event); | ||
| }); | ||
|
|
||
| return resultEvents; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| module.exports = EventMapHelper; | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really an expert in js workers, but I see that after the instantiation (
event-highlighter.js:52), thisonmessagefield is overridden. Is then this assignment useless?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, this is the function that will be triggered, when the web worker receives messages from the main thread. The other one in in
event-highlighter.js:52is the function that will be triggered, when the main thread receives a message from the web worker.This is the hole main idea. Doing the calculation and message buffer handling within a separate thread and independant from the animation frame.