Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use Span.Content. #4986

Closed
NTaylorMullen opened this issue Jan 23, 2016 · 10 comments
Closed

Don't use Span.Content. #4986

NTaylorMullen opened this issue Jan 23, 2016 · 10 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-razor-pages Perf
Milestone

Comments

@NTaylorMullen
Copy link
Contributor

Today we use Span.Content in several locations where we don't need to during parse and code gen time. Instead we should be utilizing the Symbols that are available to us to prevent additional allocations.

image

@henkmollema
Copy link
Contributor

Off-topic: but I was just wondering what profiling tool you are using and how do you measure just the code of the library and not DNX code as well.

@NTaylorMullen
Copy link
Contributor Author

I'm using dotMemory. It's quite nice because you can have DNX boot your application and then you choose when to start collecting a profile. That way you only start collecting once your app is up and running.

@Eilon
Copy link
Member

Eilon commented May 22, 2016

@NTaylorMullen need an update on this because it's been stale. Have we seen perf issues here that we need to investigate for 1.0.0? If not, I'll move this out.

@NTaylorMullen
Copy link
Contributor Author

@ToddGrun are there perf issues associated with the amount of strings we've been allocating on the Visual Studio Razor editor side of things? I haven't heard anything so I'm tempted to say post 1.0.0.

@Eilon
Copy link
Member

Eilon commented May 22, 2016

@NTaylorMullen can you email @ToddGrun about this? Might be easiest.

In the meantime, moving out.

@ToddGrun
Copy link
Contributor

@Eilon We haven't noticed any allocation issues, but we haven't done allocation profiling in razor scenarios.

@Eilon
Copy link
Member

Eilon commented May 23, 2016

@ToddGrun ok thanks!

@NTaylorMullen
Copy link
Contributor Author

In order to accomplish this change we'd need to break API compatibility. This should be pushed out until we're able to do that.

@NTaylorMullen
Copy link
Contributor Author

Ping to push out to a milestone that enables us to break API compatibility.

@Eilon
Copy link
Member

Eilon commented Jul 18, 2016

Moved to 2.x

@aspnet-hello aspnet-hello transferred this issue from aspnet/Razor Dec 14, 2018
@aspnet-hello aspnet-hello added this to the Backlog milestone Dec 14, 2018
@aspnet-hello aspnet-hello added 1 - Ready area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-razor-pages Perf labels Dec 14, 2018
@pranavkm pranavkm added the c label Aug 20, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-razor-pages Perf
Projects
None yet
Development

No branches or pull requests

8 participants