search

apache / datasketches-memory

High performance native memory access for Java.
https://datasketches.apache.org
Apache License 2.0
114 stars 27 forks source link

Question about AccessByteBuffer #22

Closed leventov closed 6 years ago

leventov commented 6 years ago

https://github.com/DataSketches/memory/blob/5356a8b1db8aaf985acb22766e803f8737de115e/src/main/java/com/yahoo/memory/AccessByteBuffer.java#L32-L80

Why read-only and writable branches are so different?

leerho commented 6 years ago

Because Java does some really strange and inconsistent things under the covers. For example, a slice offset is saved differently for a direct BB vs a heap BB. If the BB is Read-only, I have to acquire the values of hb and offset via reflection.

leventov commented 6 years ago

I understand difference between heap and direct, but I don't understand why in case of writable you don't access hb and offset.

leerho commented 6 years ago

Because I have normal access to byteBuf.array() = hb, and byteBuf.arrayOffset()= offset.

leventov commented 6 years ago

I didn't complete review, so want to keep it open as a reminder

leerho commented 6 years ago
  1. Do you still want to keep this open?
  2. Everything we have discussed so far is merged into master. Are you planning more changes?
  3. Do you need an Maven Central artifact? Or do you plan to create your own jars? If you need an artifact, when?
leventov commented 6 years ago
  1. Please keep open, not reviewed yet.
  2. I plan to add support of non-native Memory and Buffer!
  3. I will need after that
leerho commented 6 years ago

@leventov

Please review.

leerho commented 6 years ago

I am closing this due to lack of response.

leventov commented 6 years ago

Ok, I finally understood why you are doing this: because ByteBuffer.array() and arrayOffset() throw an exception when attempted to be called on a read-only BB. The other path works in both cases though so I decided to simplify the code in #73.

leerho commented 6 years ago

Yes and I agree with your simplified code.