xoofx / Tomlyn

Tomlyn is a TOML parser, validator and authoring library for .NET Framework and .NET Core
BSD 2-Clause "Simplified" License
416 stars 29 forks source link

Provide `Async` methods #42

Open JamieMagee opened 2 years ago

JamieMagee commented 2 years ago

In the same way that System.Text.Json.JsonSerializer provides SerializeAsync and DeserializeAsync, is it possible to provide Async versions of ToModel and FromModel?

xoofx commented 2 years ago

Sorry, but I will have to decline this. 🙂

Async is so viral that I don't want to duplicate the code in Tomlyn, specially when TOML is mostly used for configuration files that are a few KB. That would be important if Tomlyn was used in the context of loading MB of files in and out, but I'm quite skeptical about the use case. Also async/await is usually performing worse than their sync counterpart because it allocates all around for continuations and there are less optimizations possible because of the generated stack machines. ValueTask doesn't help much there, it saves a few allocations but not that much. See for example my blog post here about Making Scriban async compatible (spoiler: async 3x slower)

It is not pretty, but if you desperately need async/await versions, you can try:

JamieMagee commented 2 years ago

Thanks for the explanation.

Is this something you'd accept contributions for, or something you'd never accept into Tomlyn? If you'd never accept contributions for, I think you can close this issue.

xoofx commented 2 years ago

Is this something you'd accept contributions for, or something you'd never accept into Tomlyn? If you'd never accept contributions for, I think you can close this issue.

For reading, the parsing system is entirely relying on a string in memory, so it wouldn't be required to add anything. For writing that would require to duplicate the code entirely.

I would like to understand the use case. Why is it that important that you need to have an async version of Tomlyn? Couldn't you serialize to a string in memory and then serialize the string to the disk with async separately. That would be much more efficient in terms of async usage (one write, instead of an async state machine for every single TextWriter.Write)

lilith commented 8 months ago

Async adds a lot of negatives, and I can't see it being used as a heavy data-serialization format where streaming would be useful. At most I could see adding async wrappers that keep all de and serialization paths sync, but call Stream or PipeReader/PipeWriter methods in an async fashion prior to invoking the parsing or after serialization, but I don't think parsing should be async.