System.Xml.XmlCharType size improvements

[marek-safar]

System.Xml.XmlCharType has dependency on 64K big binary blob XmlCharType.bin to do xml characters type detection. This dependency is hard for illinker to remove and also looks like something what could be implement differently.

There are also static methods like IsSurrogate which are just duplicates of char methods implementation and could probably be removed completely.

[danmoseley]

@marek-safar I imagine there will be many more issues over the next few months relating to linker optimization. How about I create a github project to collect them all.

[marek-safar]

@danmosemsft yes, that's very likely. I don't know how you usually do it if via Projects, umbrella issue or something else

[danmoseley]

OK, added this to a new project. Feel free to put any others there.

[ericstj]

Seems like a good case for using this feature: dotnet/roslyn#24621

EG: dotnet/coreclr#26138

/cc @eerhardt

[vitek-karas]

I vaguely remember that in the early days of System.Xml we used to generate this table into memory on init. But it turned out to be faster to simply map it from disk.
Anyway, the code to generate it is still there:

runtime/src/libraries/System.Private.Xml/src/System/Xml/XmlCharType.cs

Line 288 in fe35942

#if XMLCHARTYPE_GEN_RESOURCE

The mapping of constants into memory won't help with the size, since we would still need the 64KB array as a constant value in the assembly - basically it would be a nicer way to do the resource thing we're doing today.

For linker we would need to do this via a feature switch and in that case drop the resource and instead use the CPU to generate it on init. Would mean to include the generation code always, but it's really small.

Technically it's possible to not have the 64KB table at all, but it REALLY helps with XML parser perf, so I think we should keep it.

[buyaa-n]

It doesn't seem to we could get it done within 5.0, moving to 6.0

[ericstj]

@eerhardt is it OK to move this or do we need it for 5.0 browser scenarios?

@vitek-karas I wasn't suggesting a constant: I was suggesting a field that could be trimmed if it wasn't used. My understanding of the issue was that the linker doesn't know how to trim a resource, but it would know how to trim a field. The fact that we can have spans of this gives us good perf on access. Do you think it's ok for perf to regress perf of XML to save 64 KB in this case? I guess it depends on how bad the perf is to generate the table in browser.

[eerhardt]

is it OK to move this or do we need it for 5.0 browser scenarios?

As of right now, System.Xml isn't brought into a Blazor WASM app by default and I don't know of important scenarios that would require XML in Blazor WASM. So I don't feel it is strictly necessary for 5.0, unless someone makes a case for why XML is critical in Blazor WASM. I think it is fine to address this in 6.0.

[vitek-karas]

@ericstj currently linker can only trim resources "unconditionally" (behind a feature switch), so you're right about that. If it's a field which linker can trim along with its large value that would be much better. Ultimately though (as far as I remember my XML days), pretty much everything in System.Xml will end up accessing this field anyway. So if we want to reduce size of apps using XML generating the table on init would be the ideal solution for size. Performance is obviously a concern - we would have to measure that.

[eerhardt]

Moving to 'Future' as this isn't a high priority for 6.