ARROW-5199:[Java] Add unsafe access methods to ArrowBuf

[yuruiz]

Add unsafe access method to ArrowBuf to allow external user to access underlying memory chunk without extra boundary check overhead

[codecov-io]

Codecov Report

Merging #4186 into master will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4186      +/-   ##
==========================================
+ Coverage   87.77%   87.83%   +0.06%     
==========================================
  Files         758      758              
  Lines       92442    92975     +533     
  Branches     1251     1251              
==========================================
+ Hits        81145    81669     +524     
- Misses      11180    11189       +9     
  Partials      117      117

Impacted Files	Coverage Δ
cpp/src/arrow/csv/options.h	75% <0%> (-25%)	⬇️
js/src/ipc/metadata/json.ts	92.39% <0%> (-4.35%)	⬇️
cpp/src/arrow/csv/column-builder.cc	95.32% <0%> (-1.76%)	⬇️
js/src/data.ts	87.87% <0%> (-0.76%)	⬇️
python/pyarrow/compat.py	89.77% <0%> (-0.71%)	⬇️
python/pyarrow/tests/test_table.py	99.61% <0%> (-0.39%)	⬇️
cpp/src/parquet/arrow/arrow-schema-test.cc	99.48% <0%> (-0.04%)	⬇️
cpp/src/arrow/util/bpacking.h	99.81% <0%> (-0.01%)	⬇️
python/pyarrow/lib.pyx	100% <0%> (ø)	⬆️
cpp/src/arrow/csv/converter-test.cc	100% <0%> (ø)	⬆️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85db347...17bfce3. Read the comment docs.

[jacques-n]

I'd like to propose that we get ARROW-3191 merged first and then change how to do this. I think it probably makes sense to simply implement an UncheckedBuf which can simply be a set of accessors along with a memory address that interacts with the underlying memory via unchecked methods. The generic can hold a reference object (which may be ArrowBuf) but could be anything else, too. Since the address is exposed in ArrowBuf, you can grab that to create an UncheckedBuf. There is no need for these methods to be directly attached to the ArrowBuf class.

[yuruiz]

I'd like to propose that we get ARROW-3191 merged first and then change how to do this. I think it probably makes sense to simply implement an UncheckedBuf which can simply be a set of accessors along with a memory address that interacts with the underlying memory via unchecked methods. The generic can hold a reference object (which may be ArrowBuf) but could be anything else, too. Since the address is exposed in ArrowBuf, you can grab that to create an UncheckedBuf. There is no need for these methods to be directly attached to the ArrowBuf class.

Sure, it makes sense to merge ARROW-3191 first. I would prefer having explicit unsafe method so that the external caller would be aware that such a method have no boundary check. If we simply providing an UncheckedBuf implementation it could be quite confusing about the semantics of each accessor.

[jacques-n]

The UncheckedBuf can have the methods be called getUnsafe(). UncheckedBuf is not a subclass of ArrowBuf, it is simply something that holds a reference to ArrowBuf.

[emkornfield]

@yuruiz can you clarify the use case for this? Does using the BoundsChecking configuration not work for you?

[yuruiz]

@yuruiz can you clarify the use case for this? Does using the BoundsChecking configuration not work for you?

HI @emkornfield , by boundsChecking flag are you referring the flag introduced by this PR ? I can update it if that is the preferred way.

[emkornfield]

That flag is for verifying a value is set when retrieving. I was referring to this which is the same thing for checking bounds on addresses.

[yuruiz]

That flag is for verifying a value is set when retrieving. I was referring to this which is the same thing for checking bounds on addresses.

That looks good. I would close this PR.