perf(gatsby): Shortcut trivial queries by id, potential huge win #20609
Conversation
|
🔥 |
|
Nice!! Well done 👏 |
|
😮 wow! amazing work! |
There was a problem hiding this comment.
This sounds fantastic! But I'd also wait for a review from @freiksenet as this is one of the most complex parts in Gatsby core and I don't have enough expertise on it yet.
| } | ||
|
|
||
| return node | ||
| } |
There was a problem hiding this comment.
This function looks a lot like addResolvedNodes. I guess it is a special case of it so maybe we could re-use common logic of those two.
There was a problem hiding this comment.
The important point is that it doesn't iterate the entire list of nodes. That's very important when scaling up. And since nodes is a Map, there's no short circuit mechanism like .some or .every. Any short cicuiting approach would mean doing a full loop through all elements, or worse (generate an array with all keys or smth, before being able to do a partial loop).
If we were to track a shadow array with all keys of the map then short circuiting would be an option. This would cut down mandatory Map induced O(n) or O(n^2) loops by half. Not relevant for big oh, but very relevant if the total runtime is "only" 2h. But that's for another time. And would still be miles worse than the O(1) operation (-> one hash lookup) the new function offers.
There's really just three steps in the new function; get nodes by type, fetch node by index, decorate node. The existing function does the first and, in a loop, the second.
While it's possible to share code, I'm not seeing a solution where the solution isn't at least as bad-if-not-worse in terms of maintanance than the existing duplication. I am open to suggestions.
|
I had jumped on a call with @freiksenet for a sanity check to confirm this before creating these PR's :) But I welcome his review. |
pvdz
force-pushed
the
real-by-id-shortcut
branch
from
ad345a8
to
3e2e62e
Compare
While there is a check for queries by `id`, this one circumvents a few more steps. It prevents having to build up an array based on type. Instead, if it sees a query by `id`, it will immediately just fetch that node directly. This makes many sites that use plain queries by id a helluvalot faster. One site that made me look into this problem went down from 4.5 hours to about 5 minutes.
pvdz
force-pushed
the
real-by-id-shortcut
branch
from
3e2e62e
to
7ee8127
Compare
While there is a check for queries by
id, this one circumvents a few more steps. It prevents having to build up an array based on type. Instead, if it sees a query byid, it will immediately just fetch that node directly.This makes many large sites that use plain queries by id a helluvalot faster. One site that made me look into this problem with 145k pages went down from 4.5 hours to about 5 minutes (that is not a typo).