Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

stephentoub
Copy link
Member

Anywhere Environment.NewLine was being used internal to corelib, instead changes it to use an internal Environment.NewLineConst, which is just a const string. The main benefit of that is places where it's then concatenated with another const allows the C# compiler to do the concat at compile time rather than at run time.

Where I was already touching the code, fixed a few places where additional strings were being created unnecessarily, e.g. s += a; s += b; will do two string.Concat calls each with two arguments, whereas s += a + b; will do just one string.Concat call with three arguments.

@EgorBo
Copy link
Member

EgorBo commented Oct 3, 2019

Shouldn't Environment.NewLine be an intrinsic instead? Like BitConverter.IsLittleEndian or string.Empty?

UPD: ah, I guess JIT is not able to concat const strings yet (but would be nice to be able to fold CNS_STR nodes, I think it's not difficult to implement: #26000 (comment))

@stephentoub
Copy link
Member Author

I guess JIT is not able to concat const strings yet (but would be nice to be able to fold CNS_STR nodes, I think it's not difficult to implement

It of course could be done, but it'd be more than just handling constant strings... the JIT would need to unwind decisions the C# compiler makes about what overloads of string.Concat to use.

@EgorBo
Copy link
Member

EgorBo commented Oct 3, 2019

I guess JIT is not able to concat const strings yet (but would be nice to be able to fold CNS_STR nodes, I think it's not difficult to implement

It of course could be done, but it'd be more than just handling constant strings... the JIT would need to unwind decisions the C# compiler makes about what overloads of string.Concat to use.

I mean at least simple cases, e.g. (from your changes):

text.Append(Environment.NewLine + InnerExceptionPrefix);

So JIT in runtime should replace Environment.NewLine with CNS_STR (const string) because its value never changes. Environment.NewLine + InnerExceptionPrefix is:

\--* CALL ref String.Concat +--* CALL ref Environment.get_NewLine \--* CNS_STR ref <string constant> 

then is converted into:

\--* CALL ref String.Concat +--* CNS_STR ref <string constant> ;; it's now a const \--* CNS_STR ref <string constant> 

then JIT should fold it into just

\--* CNS_STR ref <string constant> 
@stephentoub stephentoub merged commit 073ad7e into dotnet:master Oct 3, 2019
@stephentoub stephentoub deleted the newline branch October 3, 2019 20:40
SrivastavaAnubhav pushed a commit to SrivastavaAnubhav/coreclr that referenced this pull request Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

3 participants