Closed jamesgurung closed 3 years ago
Thanks for mentioning this. Do you want to submit a PR?
I'm afraid I don't know enough about string allocations to submit a PR. Changing the type to StringBuilder
will compile, but I'm not convinced that would be any more efficient than:
return (addLeadingSlash ? "/" : string.Empty) + templateKey.Replace('\\', '/');
Guid.NewGuid + "." + Guid.NewGuid
as your key to test )GetFullPathName
. This same function used internally by the runtime framework is also exposed as an API call: https://docs.microsoft.com/en-us/dotnet/api/system.io.path.getfullpath?view=netcore-3.1#System_IO_Path_GetFullPath_System_String_ - as the remark suggestion suggests, the downside to this is it requires file system access to compute the full path name. However, it will return a canonical path name. The downside to calling this function is that someone could change the casing of the file, and so two calls to Path.GetFullPath could return two different values for the same logical resource. (I think this is bad, so we may just want to update the code to explicitly document the reasons for not calling Path.GetFullPath).@HamiltonManalo is this something you'd be interested in fixing for .NET Core 5.0?
@jzabroski Yeah, I will start work on this as soon as I can.
For the InplaceStringBuilder replacement there is string.Create below is an article using it. https://www.stevejgordon.co.uk/creating-strings-with-no-allocation-overhead-using-string-create-csharp
Have been fighting off a flu/sinusitis this week so I wasn't active. I'll look deeper, but my initial feeling is that unless this is sending thousands of emails a minute, the allocations aren't a huge deal.
+1
Hopefully this weekend I'll be able to free up some time and focus on this so we can close the issue.
The same error seems to be coming from this point as well:
fail: Microsoft.AspNetCore.Server.Kestrel[0]
Heartbeat.OnHeartbeat
System.TypeLoadException: Could not load type 'Microsoft.Extensions.Primitives.InplaceStringBuilder' from assembly 'Microsoft.Extensions.Primitives, Version=5.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'.
at Microsoft.Net.Http.Headers.DateTimeFormatter.ToRfc1123String(DateTimeOffset dateTime, Boolean quoted)
at Microsoft.Net.Http.Headers.HeaderUtilities.FormatDate(DateTimeOffset dateTime, Boolean quoted)
at Microsoft.Net.Http.Headers.HeaderUtilities.FormatDate(DateTimeOffset dateTime)
at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.DateHeaderValueManager.SetDateValues(DateTimeOffset value)
at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.DateHeaderValueManager.OnHeartbeat(DateTimeOffset now)
at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure.Heartbeat.OnHeartbeat()
info: CoreWCF.Channels.ServiceModelHttpMiddleware[0]
This is with NET 5 RC2 today. I am not sure if this is related though... So, please, disregard if it is not.
@kkkmail A fix hasn't been implemented for this yet. I'm going to begin working on it tonight (better late than never) and it should make it into the Beta11 release.
I created a quick simple PR that changes to using StringBuilder. #380
I noticed that the fix for this has already been merged. Will the next Beta be available soon?
Template compilation now fails because the obsoleted
InplaceStringBuilder
was removed from the .NET Runtime in commit https://github.com/dotnet/runtime/commit/ef4bc4d503b71f9de1a8988da6db38bcfcfb0cc4.This causes an error on the line:
https://github.com/toddams/RazorLight/blob/8e043e10423ecc0b88b49787dd597adddd4caa31/src/RazorLight/Compilation/RazorTemplateCompiler.cs#L263