search

krzema12 / snakeyaml-engine-kmp

A Kotlin Multiplatform port of snakeyaml-engine
Apache License 2.0
28 stars 6 forks source link

Use Kotlinx.IO #212

Open mani1232 opened 4 months ago

mani1232 commented 4 months ago

https://github.com/Kotlin/kotlinx-io

krzema12 commented 4 months ago

At some point, I'd love to, but for now it's marked as "alpha" and "incubator" - doesn't look stable. Let's wait until it's at least beta?

aSemy commented 4 months ago

I haven't looked at kotlinx-io at all, but I strongly suspect (since it's in alpha) it doesn't support all of the features that we use... yet.

OptimumCode commented 4 months ago

I think it would be great to do something similar to what is done in kotlinx-serialization-json for okio and kotlin-io. They have a few abstractions like InternalJsonWriter and InternalJsonReaderCodePointImpl in kotlinx-json library and different implementations for okio and kotlin-io in separate modules. Probably, the same can be done here to support both okio and kotlin-io.

In this case, we would be able to create independent implementations for okio and kotlin-io (which can be marked as experimental) and people can choose what to use.

OptimumCode commented 4 months ago

I haven't looked at kotlinx-io at all, but I strongly suspect (since it's in alpha) it doesn't support all of the features that we use... yet.

I might be missing something but from my point of view the only things used right now from okio are:

From a first look, all of that can be found in kotlinx-io (with some different naming but it can be found)

aSemy commented 3 months ago

think it would be great to do something similar to what is done in kotlinx-serialization-json for okio and kotlin-io.

I do like this idea, but I think it makes sense for Kaml to do this, not SnakeKMP. Maybe SnakeKMP does need to abstract all of the IO stuff and rely on Kaml converting Okio/kotlinx-io types into the abstractions.

From a first look, all of that can be found in kotlinx-io (with some different naming but it can be found)

My suspicion could be wrong :) But from looking at the kotlinx-io PR for Kaml, it requires using InternalIoApi. We definitely need to avoid that.

OptimumCode commented 3 months ago

But from looking at the kotlinx-io PR for Kaml, it requires using InternalIoApi. We definitely need to avoid that.

It looks like a mistake in PR. The buffer from kotlinx-io is both Sink and Source when in okio the buffer() method returns BufferedSource. But in any case, using library's internal api should be avoided as much as possible.

Maybe SnakeKMP does need to abstract all of the IO stuff and rely on Kaml converting Okio/kotlinx-io types into the abstractions.

This idea sounds great. Creating an abstractions from a particular IO library will give more flexibility for end users. And this definitely should be done as the first step to support different IO libraries. But I still think that it would be better to have integrations with okio and kotlinx-io maintained in this repo. This would give us more insight on how good the SnakeKMP integrates with those IO libraries (by performing tests and benchmarks for everything together) and ability to define some internal API to optimize the integration.

aSemy commented 2 months ago

But I still think that it would be better to have integrations with okio and kotlinx-io maintained in this repo. This would give us more insight on how good the SnakeKMP integrates with those IO libraries (by performing tests and benchmarks for everything together) and ability to define some internal API to optimize the integration.

Don't get me wrong, I'm not disagreeing: better support for Okio and kotlinx-io would certainly be a good thing, and I'm totally on board. However, I see SnakeKMP is the supporting, utility focused backend, and Kaml is the friendly, user facing frontend. SnakeKMP should focus on performance and correctness, while Kaml should focus on usability. (Ideally SnakeKMP would have zero dependencies, but the Kotlin multiplatform stdlib doesn't have enough IO stuff yet.)

So, when it comes to SnakeKMP I think the best strategy is to pick the IO library that works best for performance and integration with Kaml. Currently I think Okio is best suited to support that, since it's older and has more support. But definitely when kotlinx-io becomes stable it makes sense to use it instead, because it's official.

And then it's up to Kaml to provide Okio/kotlinx-io/java.io/whatever-else integrations.

At least, that's how I see it!

OptimumCode commented 2 months ago

I think I got your point and I agree with you, especially with

Ideally SnakeKMP would have zero dependencies

Maybe it is worth parking this question until Kotlin has standardized IO interfaces...

But if SnakeKMP had an extension point (like kotlinx-serizalization-json does), it would allow to do something about the above right now (for Kaml or any other library/application). Of course, if this extension point causes a big performance degradation that would not be worth it.