perf: Cache decorator and camera render closures instead of allocating per frame - #3979
perf: Cache decorator and camera render closures instead of allocating per frame#3979spydon wants to merge 1 commit into
Conversation
a984917 to
f549bd5
Compare
| void renderTree(Canvas canvas) { | ||
| if (isRendered) { | ||
| _renderEffect.applyChain(super.renderTree, canvas); | ||
| _renderEffect.applyChain(_superRenderTree ??= super.renderTree, canvas); |
There was a problem hiding this comment.
it is quite unfortunate that the compiler can't optimize this
There was a problem hiding this comment.
an alternative to consider, make applyChain receive a generic component or superclass with renderTree and have it call the function directly, avoiding the lambda entirely
| @override | ||
| void renderTree(Canvas canvas) { | ||
| decorator.applyChain(super.renderTree, canvas); | ||
| decorator.applyChain(_superRenderTree ??= super.renderTree, canvas); |
There was a problem hiding this comment.
curious why all the others can't use the has decorator mixin? is there a way to share this logic upstream?
| _renderContext.postProcess = postProcess; | ||
| } | ||
|
|
||
| return (canvas) { |
There was a problem hiding this comment.
you don't want to tear-off-ify this lambda too?
| if (_next == null) { | ||
| apply(draw, canvas); | ||
| } else { | ||
| if (!identical(_chainedDrawSource, draw)) { |
There was a problem hiding this comment.
the need for this makes me question if the decorator pattern is the best one for the dart language.
the other tear-offs are ok but this is adding many more layers of complexity, state management and potential for mistakes.
but since this is a hot path for all render calls of all components (regardless of having decorators), this is ok. still trying to think more holistically in the meanwhile
2 participants
Description
The render pass allocated closures every frame: a
super.renderTreetear-off perPositionComponent(andHasDecorator/Route/PostProcessComponent) passed toDecorator.applyChain, a chain-forwarding closure insideapplyChainitself, andCameraComponent'srenderWorldlocal function per camera. These are now cached fields or instance methods.Extracted from #3960 so the data-structure change there stands alone (as requested in this comment). Stacked on #3978.
Checklist
docsand added dartdoc comments with///.examplesordocs.Breaking Change?
Related Issues
Relates to #3957