-
Notifications
You must be signed in to change notification settings - Fork 13
feat(widget): Batch tile requests within an animation frame #181
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
Conversation
HiGlass's built-in data fetching logic is designed to optimize API calls by consolidating requests based on time, ID, and server. Our custom Jupyter data fetcher doesn't include these optimizations. This PR introduces a `consolidator` helper, which performs a much simpler form of batching. Since we assume a single server, this function only batches tile requests that occur within the same animation frame and submits them together. This helps reduce the number of comms calls and deduplicates requests efficiently. Initial testing suggests it feels quite responsive!
src/higlass/widget.js
Outdated
| * @template T | ||
| * @returns {{ promise: Promise<T>, resolve: (success: T) => void, reject: (err: unknown) => void }} | ||
| */ | ||
| function defer() { |
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 can't wrap my head around what this function is supposed to do.
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.
It's a polyfill for something like Promise.withResolvers.
Basically, it lets you create a "deferred" value. Normally, you'd resolve or reject a promise directly within the promise body. But this popular technique lets you bring the resolve/reject functions into the same scope as the promise. This let s you return the promise directly (for something else to "await") and then pass around the resolvers for that promise to other internal functions and methods.
This lets us have a top level API of:
const submit = consolidator((batch) => {
for (let {data, resolve} of batch) {
resolve(data + "!");
}
});
let aPromise = submit("a");
let bPromise = submit("b");
await Promise.all([aPromise, bPromise]) // ["a!" "b!"]See how each of the callers nicely can "await" their individual submissions without needing to know about the batching. It's because we pass down the resolve for each of those separate promises to the processBatch handler.
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.
Oh, actually Promise.withResolvers is pretty widely available now according to MDN. I think I'll just use it direcly.
HiGlass's built-in data fetching logic is designed to optimize API calls
by consolidating requests based on time, ID, and server. Our custom
Jupyter data fetcher doesn't include these optimizations.
This PR introduces a
consolidatorhelper, which performs a much simplerform of batching. Since we assume a single server, this function only
batches tile requests that occur within the same animation frame and
submits them together. This helps reduce the number of comms calls, and
at a minimum dedupes requests with the same tile id.
This is the approach taken in mosaic:
Initial testing suggests it feels quite responsive!
Before (slight lag due to repeated (shared) requests)
After
Checklist
fix:orfeat:to help organize auto-generatednotes.