Hi Liya Fan, Based on the Float8Benchmark there does not seem to be any meaningful performance difference on my machine. At least for me, the benchmarks are not stable enough to say one is faster than the other (I've pasted results below). That being said my machine isn't necessarily the most reliable for benchmarking.
On an intuitive level, this makes sense to me, for the most part it seems like the change just moves casting from "int" to "long" further up the stack for "PlatformDepdendent" operations. If there are other benchmarks that you think are worth running let me know. One downside performance wise I think for his change is it increases the size of ArrowBuf objects, which I suppose could influence cache misses at some level or increase the size of call-stacks, but this doesn't seem to show up in the benchmark.. Thanks, Micah Sample benchmark numbers: [New Code] Benchmark Mode Cnt Score Error Units Float8Benchmarks.copyFromBenchmark avgt 5 15.441 ± 0.469 us/op Float8Benchmarks.readWriteBenchmark avgt 5 14.057 ± 0.115 us/op [Old code] Benchmark Mode Cnt Score Error Units Float8Benchmarks.copyFromBenchmark avgt 5 16.248 ± 1.409 us/op Float8Benchmarks.readWriteBenchmark avgt 5 14.150 ± 0.084 us/op On Tue, Aug 6, 2019 at 1:18 AM Fan Liya <liya.fa...@gmail.com> wrote: > Hi Micah, > > Thanks a lot for doing this. > > I am a little concerned about if there is any negative performance impact > on the current 32-bit-length based applications. > Can we do some performance comparison on our existing benchmarks? > > Best, > Liya Fan > > > On Tue, Aug 6, 2019 at 3:35 PM Micah Kornfield <emkornfi...@gmail.com> > wrote: > >> There have been some previous discussions on the mailing about supporting >> 64-bit lengths for Java ValueVectors (this is what the IPC specification >> and C++ support). I created a PR [1] that changes all APIs that I could >> find that take an index to take an "long" instead of an "int" (and >> similarly change "size/rowcount" APIs). >> >> It is a big change, so I think it is worth discussing if it is something >> we >> still want to move forward with. It would be nice to come to a conclusion >> quickly, ideally in the next few days, to avoid a lot of merge conflicts. >> >> The reason I did this work now is the C++ implementation has added support >> for LargeList, LargeBinary and LargeString arrays and based on prior >> discussions we need to have similar support in Java before our next >> release. Support 64-bit indexes means we can have full compatibility and >> make the most use of the types in Java. >> >> Look forward to hearing feedback. >> >> Thanks, >> Micah >> >> [1] https://github.com/apache/arrow/pull/5020 >> >
