Rewrite Blob to facilitate interop with other libs #1211
Conversation
Rewrite the structure of Blob to help interop with other libraries and patterns, such as fs2 or scodec.
|
@armanbilge, if you could spare the time to give me you your opinion on this, it'd be well appreciated |
There was a problem hiding this comment.
Seems fine. FWIW I think you could get away with just ArraySliceBlob and QueueBlob (also I think JVM special-cases classes with no more than 2 implementations, but don't quote me).
Many ByteBuffers are backed by Array[Byte]s so you can convert them to ArraySliceBlob without copying. In the case where they're not (e.g. off-heap buffer), you'd have to copy to bring it into the ADT, but chances are you'd have to copy at some point anyway. The only case you lose is when you are literally just shoveling bytes between sockets/files without any parsing/transformation, then keeping things in off-heap ByteBuffers could win. Edit: note that FS2 doesn't use off-heap buffers anyway, actually they are kind of hostile to immutable datatypes since they don't GC well.
* Remove ByteArrayBlob (superseded by ArraySiceBlob) * Add an `toArraySliceBlob` interface method * Make class constructors private * Add smart constructors
modules/core/src/smithy4s/Blob.scala
Outdated
| hashCode += bytes(i).hashCode() | ||
| i += 1 | ||
| override def asByteBufferUnsafe(offset: Int, size: Int): ByteBuffer = { | ||
| val b = buf |
There was a problem hiding this comment.
ByteBuffers are really annoying and hard to work correctly with. This is why I'm skeptical if its worth to give them a designated position in the ADT.
| val b = buf | |
| val b = buf.duplicate() |
There was a problem hiding this comment.
Hmmm 😅 I now realize this is possibly my own mistake in scodec-bits. I need to think about this.
There was a problem hiding this comment.
Yup, this was pretty much copied verbatim from scodecs-bits.
There was a problem hiding this comment.
There was a problem hiding this comment.
Addressed in aabf826
| try { | ||
| val _ = data(6) | ||
| fail("expected exception") | ||
| } catch { | ||
| case _: IndexOutOfBoundsException => () | ||
| } |
There was a problem hiding this comment.
| try { | |
| val _ = data(6) | |
| fail("expected exception") | |
| } catch { | |
| case _: IndexOutOfBoundsException => () | |
| } | |
| intercept[IndexOutOfBoundsException]{ data(6) } |
|
|
||
| test(s"$name: copyToStream") { | ||
| val stream = new ByteArrayOutputStream() | ||
| try { |
There was a problem hiding this comment.
the other day I discovered Using and Using.resource to do this kind of resource management
There was a problem hiding this comment.
Thanks. I was under the impression it was a 2.13+ only thing
| val _ = buffer.get(arr) | ||
| } | ||
| override def hashCode(): Int = { | ||
| import util.hashing.MurmurHash3 |
Rewrite the structure of Blob to help interop with other libraries and patterns, such as fs2 or scodec.
Inspired from :