Pool TagHelperExecutionContexts within TagHelperScopeManager.
#695
Conversation
| } | ||
|
|
||
| // Used to pool TagHelperExecutionContext's. With this we don't always need to recreate TagHelperExecutionContexts. | ||
| internal void ClearAndUpdateWith( |
There was a problem hiding this comment.
Why internal? Would user code ever see this type?
There was a problem hiding this comment.
Nope, user code wont see this type. As for Reset, that was my first naming of this but felt that the name mislead with what it really did.
There was a problem hiding this comment.
Suggest Reinitialize(). Also suggest public because internal is used only for bits that would be private if not for testing. (All of Microsoft.AspNetCore.Razor.Runtime.TagHelpers could be named .Internal if we wanted.)
NTaylorMullen
force-pushed
the
nimullen/poolthcontexts.674
branch
from
65d8462 to
1e73a71
Compare
|
🆙 📅 |
| /// <param name="items">The <see cref="IDictionary{Object, Object}"/> to use.</param> | ||
| /// <param name="uniqueId">The unique id to use.</param> | ||
| /// <param name="executeChildContentAsync">The <see cref="Func{Task}"/> to use.</param> | ||
| public void Reinitialize( |
There was a problem hiding this comment.
Suggest removing overlapping parameters from the constructors, requiring a Reinitialize() call for all uses. Alternatively call Reinitialize() from the constructors duplicating this code.
|
⌚ (looking very solid) |
|
🆙 📅 |
| throw new ArgumentNullException(nameof(endTagHelperWritingScope)); | ||
| } | ||
|
|
||
| Reinitialize(tagName, tagMode, items, uniqueId, executeChildContentAsync); |
There was a problem hiding this comment.
Call this after initializing _tagHelpers and just .Clear() that field in Reinitialize().
|
Not urgent to check but am curious whether |
|
|
w/ a couple of small fixes- Currently the `TagHelperScopeManager` creates a new `TagHelperExecutionContext` per `TagHelper` on a given page. With this change the max number of `TagHelperExecutionContext`s per page is the number of nested levels that exist. - Added two tests validating that specific pieces of `TagHelperExecutionContext` are updated as expected. #674
NTaylorMullen
force-pushed
the
nimullen/poolthcontexts.674
branch
from
d902fe4 to
78451ef
Compare
5 participants
TagHelperScopeManagercreates a newTagHelperExecutionContextperTagHelperon a given page. With this change the max number ofTagHelperExecutionContexts per page is the number of nested levels that exist.TagHelperExecutionContextare updated as expected.Before:
After:
Result: 3% overall reduction in allocations.
#674