This repository has been archived by the owner on Dec 19, 2018. It is now read-only.

Make `TagHelperOutput` lazy initialize `TagHelperContent` properties. #358

Closed

NTaylorMullen opened this issue Apr 14, 2015 · 7 comments

Assignees

Labels

Milestone

Member

NTaylorMullen commented Apr 14, 2015

The initial implementations of the "new" TagHelperOutput data structure (one with buffers instead of strings) was originally lazy with the buffers construction but was then removed due to the added complexity via code review. We should analyze its perf impact to determine if being lazy about construction of PreElement, PreContent, Content, PostContent and PostElement's TagHelperContent objects is beneficiary.

The text was updated successfully, but these errors were encountered:

NTaylorMullen added the Perf label

NTaylorMullen mentioned this issue

Add PreElement and PostElement to TagHelperOutput. #355

Closed

danroth27 added this to the 4.0.0-beta5 milestone

danroth27 assigned NTaylorMullen

danroth27 added the 1 - Ready label

NTaylorMullen modified the milestones: 4.0.0-beta6, 4.0.0-beta5

Eilon modified the milestones: 4.0.0-beta6, 4.0.0-beta7

Eilon assigned sornaks and unassigned NTaylorMullen

Eilon mentioned this issue

TagHelperExecutionContext must be smart about new TagHelper Content allocations. #317

Closed

Member

Eilon commented Aug 12, 2015

Please see #317 for additional info.

Eilon modified the milestones: 4.0.0-beta8, 4.0.0-beta7

Eilon unassigned sornaks

Eilon modified the milestones: 4.0.0-beta8, 4.0.0-rc1

Eilon modified the milestones: 4.0.0-rc1, 4.0.0-rc2

Eilon assigned NTaylorMullen

Member

Eilon commented Oct 30, 2015

Please have a chat with @rynowak before starting the perf investigation.

Member

rynowak commented Oct 30, 2015

beneficiary

Yes. Let's discuss

Member

Eilon commented Oct 30, 2015

Hah!

Member

rynowak commented Nov 10, 2015

Let's discuss

This is definitely worth addressing

NTaylorMullen added 2 - Working and removed 1 - Ready labels

Member Author

NTaylorMullen commented Nov 13, 2015

Upon investigating a proper fix for this issue I decided to investigate what it'd take to have TagHelperOutput be an IHtmlContent (since it's responsible for lazily initializing buffers). After a lot of playing around with the idea and talks with @DamianEdwards it definitely fits.

This will also involve removing the WriteTagHelperAsync method from Mvc.

NTaylorMullen added a commit that referenced this issue


          Make TagHelperOutput an IHtmlContent.

- This allows users to write `TagHelperOutput` directly to an `IHtmlContent` accepting `TextWriter`.
- This also enables us to inspect backing fields for all of the various contents to lazily initialize them.

#358

NTaylorMullen added a commit that referenced this issue


          Lazily initialize TagHelperOutputs Content properties.

2c4e531

- Added backing fields to each of the content properties and added null propagation checks throughout the class.

#358

NTaylorMullen added a commit that referenced this issue


          Lazily initialize TagHelperOutputs Content properties.

cef6f2d

- Added backing fields to each of the content properties and added null propagation checks throughout the class.

#358

NTaylorMullen added a commit to aspnet/Mvc that referenced this issue


          React to aspnet/Razor#358.

36630e9

- Removed `WriteTagHelperAsync` methods from `RazorPage`.
- Moved `WriteTagHelperAsync` tests into Razor since `TagHelperOutput` is now an `IHtmlContent`.
- Updated code generation test files.

aspnet/Razor#358

NTaylorMullen added a commit that referenced this issue


          Make TagHelperOutput an IHtmlContent.

- This allows users to write `TagHelperOutput` directly to an `IHtmlContent` accepting `TextWriter`.
- This also enables us to inspect backing fields for all of the various contents to lazily initialize them.

#358

NTaylorMullen added a commit that referenced this issue


          Lazily initialize TagHelperOutputs Content properties.

b628ecf

- Added backing fields to each of the content properties and added null propagation checks throughout the class.

#358

NTaylorMullen added a commit that referenced this issue


          Update code generation tests to account for new TagHelperOutput.

11132c0

- Add `TagHelperOutput` unit tests.

#358

NTaylorMullen mentioned this issue

Make TagHelperOutput an IHtmlContent to lazily initialize TagHelperOutputs Content properties. #615

Merged

NTaylorMullen added a commit that referenced this issue


          Make TagHelperOutput an IHtmlContent.

abb308a

- This allows users to write `TagHelperOutput` directly to an `IHtmlContent` accepting `TextWriter`.
- This also enables us to inspect backing fields for all of the various contents to lazily initialize them.

#358

NTaylorMullen added a commit that referenced this issue


          Lazily initialize TagHelperOutputs Content properties.

cf02aaf

- Added backing fields to each of the content properties and added null propagation checks throughout the class.

#358

NTaylorMullen added a commit that referenced this issue


          Update code generation tests to account for new TagHelperOutput.

- Add `TagHelperOutput` unit tests.

#358

NTaylorMullen added a commit to aspnet/Mvc that referenced this issue


          React to aspnet/Razor#358.

5aea837

- Removed `WriteTagHelperAsync` methods from `RazorPage`.
- Moved `WriteTagHelperAsync` tests into Razor since `TagHelperOutput` is now an `IHtmlContent`.
- Updated code generation test files.

aspnet/Razor#358

NTaylorMullen mentioned this issue

React to aspnet/Razor#358. aspnet/Mvc#3582

Merged

NTaylorMullen added a commit that referenced this issue


          Make TagHelperOutput an IHtmlContent.

6c78cd1

- This allows users to write `TagHelperOutput` directly to an `IHtmlContent` accepting `TextWriter`.
- This also enables us to inspect backing fields for all of the various contents to lazily initialize them.

#358

NTaylorMullen added a commit that referenced this issue


          Lazily initialize TagHelperOutputs Content properties.

274b663

- Added backing fields to each of the content properties and added null propagation checks throughout the class.

#358

NTaylorMullen added a commit that referenced this issue


          Update code generation tests to account for new TagHelperOutput.

d7fa30d

- Add `TagHelperOutput` unit tests.

#358

NTaylorMullen added a commit that referenced this issue


          Make TagHelperOutput an IHtmlContent.

bdf869c

- This allows users to write `TagHelperOutput` directly to an `IHtmlContent` accepting `TextWriter`.
- This also enables us to inspect backing fields for all of the various contents to lazily initialize them.

#358

NTaylorMullen added a commit that referenced this issue


          Lazily initialize TagHelperOutputs Content properties.

e8327ee

- Added backing fields to each of the content properties and added null propagation checks throughout the class.

#358

NTaylorMullen added a commit that referenced this issue


          Update code generation tests to account for new TagHelperOutput.

6b43ebc

- Add `TagHelperOutput` unit tests.

#358

NTaylorMullen added a commit to aspnet/Mvc that referenced this issue


          React to aspnet/Razor#358.

a91c5d1

- Removed `WriteTagHelperAsync` methods from `RazorPage`.
- Moved `WriteTagHelperAsync` tests into Razor since `TagHelperOutput` is now an `IHtmlContent`.
- Updated code generation test files.

aspnet/Razor#358

NTaylorMullen added a commit to aspnet/Mvc that referenced this issue


          React to aspnet/Razor#358.

2dc13b5

- Removed `WriteTagHelperAsync` methods from `RazorPage`.
- Moved `WriteTagHelperAsync` tests into Razor since `TagHelperOutput` is now an `IHtmlContent`.
- Updated code generation test files.

aspnet/Razor#358

NTaylorMullen added 3 - Done and removed 2 - Working labels

Member Author

NTaylorMullen commented Nov 20, 2015

bdf869c
e8327ee
6b43ebc
aspnet/Mvc@2dc13b5

NTaylorMullen closed this as completed

NTaylorMullen changed the title ~~Analyze performance impact of TagHelperContent not being lazy on TagHelperOutput properties.~~ Make TagHelperOutput lazy initialize TagHelperContent properties.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.