xoofx/TurboXml

Default implementations on IXmlReadHandler are a footgun

Opened this issue · 3 comments

This is a great library! I have a need to parse 100+ MB of XML, most of which are various IDs that I never need to surface as a System.String, so I almost wrote my own XML parser, but I'm glad I didn't need to.

One minor annoyance are the default implementations on IXmlReadHandler. VS doesn't seem to offer the option to "Implement interface explicitly", so I had to list/implement each of these myself. If any of them is left unimplemented, the .NET runtime will need to box the implementing struct to call the default implementation provided by the interface (the type of this within the interface is a reference type). I'd honestly get rid of the default implementation, they are a footgun for perf-sensitive code where the interface is expected to be implemented on a struct. Maybe leave the one for the error case.

the .NET runtime will need to box the implementing struct to call the default implementation provided by the interface (the type of this within the interface is a reference type)

But IXmlReadHandler is used through generics in TurboXml and default implementation seems to be correctly inlined without boxing (For example here), and none of the default implementations are using this.

Do you have an example where you would get this boxing with TurboXml?

I avoid them in general. There's definitely a box in the IL sense and whether the JIT can optimize the box out depends on many things (e.g. if the struct is generic over a reference type, it would probably not optimize it out, see e.g. dotnet/runtime#39419).

Got it. I'll change this in a breaking version.