Reduce allocation in TextWriter.NewLine

[stephentoub]

It's reasonably common for a codebase to pick a specific newline string and set it regardless of OS, but currently that ends up allocating on every OS each call to set_NewLine.

Two changes:

1. When setting TextWriter.NewLine to the existing value, make it a nop to avoid unnecessarily calling ToCharArray.
2. When setting TextWriter.NewLine to "\n" on Windows or to "\r\n" on Unix, use a cached array.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull Request Overview

This PR optimizes TextWriter.NewLine setter to reduce allocations in common scenarios where codebases set a specific newline string regardless of OS. The changes include:

1. Making the setter a no-op when setting to the same value
2. Caching arrays for "\n" and "\r\n" (the non-default OS newline strings)

[GerardSmit]

Maybe a switch expression to improve readability?

Suggested change

	value == Environment.NewLineConst ? s_coreNewLine : // current OS default
	Environment.NewLineConst == "\r\n" && value == "\n" ? (s_otherCoreNewLine ??= ['\n']) : // other OS default
	Environment.NewLineConst == "\n" && value == "\r\n" ? (s_otherCoreNewLine ??= ['\r', '\n']) : // other OS default
	value.ToCharArray(); // unknown
	CoreNewLine = (value, Environment.NewLineConst) switch
	{
	// current OS default
	(Environment.NewLineConst, Environment.NewLineConst) => s_coreNewLine,
	// other OS default
	("\n", "\r\n") => s_otherCoreNewLine ??= ['\n'],
	("\r\n", "\n") => s_otherCoreNewLine ??= ['\r', '\n'],
	// unknown
	_ => value.ToCharArray(),
	};

[stephentoub]

The way I wrote it, the C# compiler will detect that some of the branches are dead and dead-code eliminate them, e.g. on Linux, the line that starts with Environment.NewLineConst == "\r\n" gets entirely removed by the C# compiler (because it sees "\n" == "\r\n"). That doesn't currently happen with the switch variant.

[GerardSmit]

Ah, I see. With tuples Roslyn doesn't see that the branche is dead code.

With the following it does Sharplab / DiffChecker.

Suggested change

	value == Environment.NewLineConst ? s_coreNewLine : // current OS default
	Environment.NewLineConst == "\r\n" && value == "\n" ? (s_otherCoreNewLine ??= ['\n']) : // other OS default
	Environment.NewLineConst == "\n" && value == "\r\n" ? (s_otherCoreNewLine ??= ['\r', '\n']) : // other OS default
	value.ToCharArray(); // unknown
	CoreNewLine = value switch
	{
	// other OS default
	"\n" when Environment.NewLineConst == "\r\n" => s_otherCoreNewLine ??= ['\n'],
	"\r\n" when Environment.NewLineConst == "\n" => s_otherCoreNewLine ??= ['\r', '\n'],
	// current OS default
	Environment.NewLineConst => s_coreNewLine,
	// unknown
	_ => value.ToCharArray(),
	};

[stephentoub]

Yup, though I personally don't think that improves readability.

[EgorBo]

That doesn't currently happen with the switch variant.

File an issue?