feat: rework child rendering to use class #10624
Conversation
|
github-actions
bot
added
the
pkg: astro
|
!bench render |
PR BenchmarkRender
Main BenchmarkRender
|
There was a problem hiding this comment.
Awesome! The results look great and I think this is worth the change. I tried to improve the rendering part too but didn't get far (#9720)
the whole renderChild buffering is interesting. thanks to the buffering, we do a lot of garbage collection (which slows execution). however, its probably still faster than doing them in serial.
i do wonder if we can rework to avoid allocating/processing individual buffers per child
Do you mean sharing a single buffer between all children? It could be faster but I'm not sure how to queue the second buffer-part until the first is done that way. It's not possible to know the size of a buffer-part ahead of time.
|
Basically we spend a lot of time waiting for garbage collection each time we finish a Mostly because we discard the buffer and the promise we were waiting for. By itself, that isn't much. But when you have thousands of promises in memory, the clean up of them takes a while I feel like there's a solution that will just click one day in my head 😅 something to do with having one big queue and sorting it at the end before flushing it |
Quite a lot of garbage collection happens when rendering due to the fact we create anonymous functions when buffering writes (the `catch`, the destination/write function, etc). By extracting this logic into a class, we can instead rely on prototype methods which exist once in memory.
Removes the `__` prefix of the private properties to match the conventions used elsewhere in the repo.
|
have updated the private properties and rebased onto main FYI |
|
This looks great, thanks so much @43081j ! |
Quite a lot of garbage collection happens when rendering due to the fact we create anonymous functions when buffering writes (the
catch, the destination/write function, etc).By extracting this logic into a class, we can instead rely on prototype methods which exist once in memory.
Testing
Existing tests should cover this.
Manual testing has happened via the benchmark script to see if any benefit was gained.
Before:
After:
Docs
N/A
Notes to reviewers
This isn't a huge gain so i'm happy to throw it away if you think the numbers are lying, or the code is worse.
the whole
renderChildbuffering is interesting. thanks to the buffering, we do a lot of garbage collection (which slows execution). however, its probably still faster than doing them in serial.i do wonder if we can rework to avoid allocating/processing individual buffers per child