search

clj-commons / byte-streams

A Rosetta stone for JVM byte representations
417 stars 33 forks source link

Use InputStream#transferTo? #35

Closed danielcompton closed 2 years ago

danielcompton commented 6 years ago

Java 9 has a new method on the InputStream interface: transferTo to transfer to an OutputStream. I don't know about it's efficiency compared to what's already here, but thought it might be a useful addition to byte-streams? There is also readAllBytes which could be useful too.

ztellman commented 6 years ago

I don’t think I can reasonably break backwards compatibility to Java 8 for a while. On Tue, May 29, 2018 at 6:10 PM Daniel Compton notifications@github.com wrote:

Java 9 has a new method on the InputStream interface: transferTo. I don't know about it's efficiency, but thought it might be a useful addition to byte-streams?

https://docs.oracle.com/javase/9/docs/api/java/io/InputStream.html#transferTo-java.io.OutputStream-

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ztellman/byte-streams/issues/35, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB6P-J758x1IuK2RYKpPDzs2Tla81Hiks5t3fGKgaJpZM4UScWY .

arnaudgeiser commented 2 years ago

transferTo is quite inflexible in terms of API. Under the hood, it relies on a 8192 bytes byte-array. [1] It doesn't correspond to the default we have on byte-streams (4096 bytes). I would say that's unlikely we'll be able to leverage this method to define the conversion between InputStream and OuputStream even if we could today consider dropping Java 8 support.

What do you think @KingMob?

[1] : https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/io/InputStream.java#L785-L795

KingMob commented 2 years ago

It looks like our existing transfer fn, is doing almost the same thing as .transferTo under the hood. They both .read() into a byte array, and .write() that to the OutputStream.

The array size would probably benefit from some micro-tuning for modern page sizes or some such, but I doubt it's a radical difference (or at least, I sure hope so...)

.readAllBytes would be convenient, but we can never guarantee a file would fit in memory, so we have to stick with reading/writing in chunks.

I think we can close this, but, @danielcompton, feel free to reopen it if you think we're missing something.