[rfc][Run Job Perf] Receive logs in webworker and send to main thread using Transferable #8734
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
salazarm
changed the title
There was a problem hiding this comment.
This looks great so far! I added one inline comment, overall it's super cool this is working! I'm not 100% up-to-date on our Apollo Client configuration, but it's possible that to deploy this we'd also need to copy some base path / auth token type of stuff that is applied in cloud? @hellendag might know more about that.
Maybe we should try to layer on one more phase of this with a perf win before merging it?
| const char = String.fromCharCode(bytes[i]); | ||
| if (char !== '\x00') { | ||
| binary += char; | ||
| } | ||
| } |
There was a problem hiding this comment.
Hmm this seems like it could be a pretty big bottleneck, is there a particular field in our JSON that contains null characters? It seems like removing them during both serialization and deserialization is unnecessary. (Nice for completeness, but if they're always used together it'd be safe to clean only the serializer?)
If we could reduce this entire function to this line, I think it'd be a big win because we can avoid having to concatenate onto a string over and over:
String.fromCharCode.apply(null, new Uint16Array(buf))
There was a problem hiding this comment.
I had that originally but hit Maximum call stack exceeded because there is a limit on the number of arguments =. I can try chunking into smaller segments. The issue I saw was that the end of the JSON had tons of null characters for some reason. I need to investigate where its coming from, this is more of a bandaid for that issue
Summary & Motivation
On heavy jobs, receiving logs for the
PipelineRunLogsSubscriptionends up taking a lot of CPU cycles on the main thread leading to poor user experience due to lack of idle time. To combat this we move the websocket to a webworker so that the data is received in a separate thread. We then transfer the data to the main-thread using anArrayBufferand parse the JSON on the main-thread. I don't expect any performance benefits from this alone but we can get some down the line by throttling the sending of messages to the main thread and by never sending some messages to the main thread if they would never be seen.Implementation:
The webworker passes websocket messages straight to the main thread as soon as it receives them.
I opted to copy the graphql subscription / fragments into a new file so that the worker doesn't have a dependency on modules with side effects that break the WebWorker (eg. modules assuming access to window/document (SharedToaster, CodeMirror, domUtils, etc.).
Right now the worker will only be enabled if a query parameter
worker=1is in the URL.How I Tested These Changes
worker:
No worker:
