hi Micah -- it makes sense to limit the scope for the time being to permitting LargeString/Binary work to proceed. Jacques, have you had a chance to look at this?
On Fri, Nov 15, 2019 at 3:07 AM Micah Kornfield <emkornfi...@gmail.com> wrote: > > Apologies for the long delay, I chose to do the minimal work of limiting this > change [1] to allowing ArrowBuf to 64-bit lengths. This would unblock work > on LargeString and LargeBinary. If this change looks OK, I think there is > some follow-up work to add more thorough unit/integration tests. > > As an aside, it does seem like the 2GB limit is affecting some users in Spark > [2][3], so hopefully LargeString would help with this. > > Allowing more than MAX_INT elements is Vectors/array still a blocker for > making LargeList useful. > > Thanks, > Micah > > [1] https://github.com/apache/arrow/pull/5020 > [2] > https://stackoverflow.com/questions/58739888/spark-is-it-possible-to-increase-pyarrow-buffer#comment103812119_58739888 > [3] https://issues.apache.org/jira/browse/ARROW-4890 > > On Fri, Aug 23, 2019 at 8:33 AM Jacques Nadeau <jacq...@apache.org> wrote: >> >> >> >> On Fri, Aug 23, 2019, 8:55 PM Micah Kornfield <emkornfi...@gmail.com> wrote: >>>> >>>> The vector indexes being limited to 32 bits doesn't limit the addressing >>>> to 32 bit chunks of memory. For example, you're prime example before was >>>> image data. Having 2 billion images of 1mb images would still be supported >>>> without changing the index addressing. >>> >>> This might be pre-coffee math, but I think we are limited to approximately >>> 2000 images because an ArrowBuf only holds up-to 2 billion bytes [1]. >>> While we have plenty of room for the offsets, we quickly run out of >>> contiguous data storage space. For LargeString and LargeBinary this could >>> be fixed by changing ArrowBuf. >>> >>> LargeArray faces the same problem only it applies to its child vectors. >>> Supporting LargeArray properly is really what drove the large-scale >>> interface change. >> >> >> My expressed concern about these changes was specifically about the use of >> long for get/set in the vector interfaces. I'm not saying that we constrain >> memory/ArrowBufs to 32bits. >> >>> >>>> Rebase would help if possible. >>> >>> I'll try to get to this in the next few days. >>> >>> [1] >>> https://github.com/apache/arrow/blob/95175fe7cb8439eebe6d2f6e0495f551d6864380/java/memory/src/main/java/io/netty/buffer/ArrowBuf.java#L164 >>> >>> On Fri, Aug 23, 2019 at 4:55 AM Jacques Nadeau <jacq...@apache.org> wrote: >>>> >>>> >>>> >>>> On Fri, Aug 23, 2019, 11:49 AM Micah Kornfield <emkornfi...@gmail.com> >>>> wrote: >>>>>> >>>>>> I don't think we should couple this discussion with the implementation >>>>>> of large list, etc since I think those two concepts are independent. >>>>> >>>>> I'm still trying to balance in my mind which is a worse experience for >>>>> consumers of the libraries for these types. Claiming that Java supports >>>>> these types but throwing an exception when the Vectors exceed 32-bits or >>>>> just say they aren't supported until we have 64-bit support in Java. >>>> >>>> >>>> The vector indexes being limited to 32 bits doesn't limit the addressing >>>> to 32 bit chunks of memory. For example, you're prime example before was >>>> image data. Having 2 billion images of 1mb images would still be supported >>>> without changing the index addressing. >>>> >>>> >>>>> >>>>>> >>>>>> I've asked some others on my team their opinions on the risk here. I >>>>>> think we should probably review some our more complex vector >>>>>> interactions and see how the jvm's assembly changes with this kind of >>>>>> change. Using microbenchmarking is good but I think we also need to see >>>>>> whether we're constantly inserting additional instructions or if in most >>>>>> cases, this actually doesn't impact instruction count. >>>>> >>>>> >>>>> Is this something that your team will take on? >>>> >>>> >>>> >>>> Yeah, we need to look at this I think. >>>> >>>>> Do you need a rebased version of the PR or is the existing one sufficient? >>>> >>>> >>>> Rebase would help if possible. >>>> >>>> >>>>> >>>>> Thanks, >>>>> Micah >>>>> >>>>> On Thu, Aug 22, 2019 at 8:56 PM Jacques Nadeau <jacq...@apache.org> wrote: >>>>>> >>>>>> I don't think we should couple this discussion with the implementation >>>>>> of large list, etc since I think those two concepts are independent. >>>>>> >>>>>> I've asked some others on my team their opinions on the risk here. I >>>>>> think we should probably review some our more complex vector >>>>>> interactions and see how the jvm's assembly changes with this kind of >>>>>> change. Using microbenchmarking is good but I think we also need to see >>>>>> whether we're constantly inserting additional instructions or if in most >>>>>> cases, this actually doesn't impact instruction count. >>>>>> >>>>>> >>>>>> >>>>>> On Wed, Aug 21, 2019 at 12:18 PM Micah Kornfield <emkornfi...@gmail.com> >>>>>> wrote: >>>>>>>> >>>>>>>> >>>>>>>> With regards to the reference implementation point. It is a good >>>>>>>> point. I'm on vacation this week. Unless you're pushing hard on this, >>>>>>>> can we pick this up and discuss more next week? >>>>>>> >>>>>>> >>>>>>> Hi Jacques, I hope you had a good rest. Any more thoughts on the >>>>>>> reference implementation aspect of this? >>>>>>> >>>>>>>> >>>>>>>> To copy the sentiments from the 0.15.0 release thread, I think it >>>>>>>> would be best to decouple this discussion from the release timeline >>>>>>>> given how many people we have relying on regular releases coming out. >>>>>>>> We can keep continue making major 0.x releases until we're ready to >>>>>>>> release 1.0.0. >>>>>>> >>>>>>> >>>>>>> I'm OK with it as long as other stakeholders are. Timed releases are >>>>>>> the way to go. As stated on the release thread [1] we need a better >>>>>>> mechanism to avoid this type of issue arising again. The release >>>>>>> thread also had some more discussion on compatibility. >>>>>>> >>>>>>> Thanks, >>>>>>> Micah >>>>>>> >>>>>>> [1] >>>>>>> https://lists.apache.org/thread.html/d70feeceaf2570906ade117030b29887af7c77ca5c4a976e6d555920@%3Cdev.arrow.apache.org%3E >>>>>>> >>>>>>> >>>>>>> On Wed, Aug 14, 2019 at 3:23 PM Wes McKinney <wesmck...@gmail.com> >>>>>>> wrote: >>>>>>>> >>>>>>>> On Sun, Aug 11, 2019 at 9:40 PM Micah Kornfield >>>>>>>> <emkornfi...@gmail.com> wrote: >>>>>>>> > >>>>>>>> > Hi Wes and Jacques, >>>>>>>> > See responses below. >>>>>>>> > >>>>>>>> > With regards to the reference implementation point. It is a good >>>>>>>> > point. I'm >>>>>>>> > > on vacation this week. Unless you're pushing hard on this, can we >>>>>>>> > > pick this >>>>>>>> > > up and discuss more next week? >>>>>>>> > >>>>>>>> > >>>>>>>> > Sure thing, enjoy your vacation. I think the only practical >>>>>>>> > implications >>>>>>>> > are it delays choices around implementing LargeList, LargeBinary, >>>>>>>> > LargeString in Java, which in turn might push out the 0.15.0 release. >>>>>>>> > >>>>>>>> >>>>>>>> To copy the sentiments from the 0.15.0 release thread, I think it >>>>>>>> would be best to decouple this discussion from the release timeline >>>>>>>> given how many people we have relying on regular releases coming out. >>>>>>>> We can keep continue making major 0.x releases until we're ready to >>>>>>>> release 1.0.0. >>>>>>>> >>>>>>>> > My stance on this is that I don't know how important it is for Java >>>>>>>> > to >>>>>>>> > > support vectors over INT32_MAX elements. The use cases enabled by >>>>>>>> > > having very large arrays seem to be concentrated in the native code >>>>>>>> > > world (e.g. C/C++/Rust) -- that could just be >>>>>>>> > > implementation-centrism >>>>>>>> > > on my part, though. >>>>>>>> > >>>>>>>> > >>>>>>>> > A data point against this view is Spark has done work to eliminate >>>>>>>> > 2GB >>>>>>>> > memory limits on its block sizes [1]. I don't claim to understand >>>>>>>> > the >>>>>>>> > implications of this. Bryan might you have any thoughts here? I'm >>>>>>>> > OK with >>>>>>>> > INT32_MAX, as well, I think we should think about what this means for >>>>>>>> > adding Large types to Java and implications for reference >>>>>>>> > implementations. >>>>>>>> > >>>>>>>> > Thanks, >>>>>>>> > Micah >>>>>>>> > >>>>>>>> > [1] https://issues.apache.org/jira/browse/SPARK-6235 >>>>>>>> > >>>>>>>> > On Sun, Aug 11, 2019 at 6:31 PM Jacques Nadeau <jacq...@apache.org> >>>>>>>> > wrote: >>>>>>>> > >>>>>>>> > > Hey Micah, >>>>>>>> > > >>>>>>>> > > Appreciate the offer on the compiling. The reality is I'm more >>>>>>>> > > concerned >>>>>>>> > > about the unknowns than the compiling issue itself. Any time >>>>>>>> > > you've been >>>>>>>> > > tuning for a while, changing something like this could be totally >>>>>>>> > > fine or >>>>>>>> > > cause a couple of major issues. For example, we've done a very >>>>>>>> > > large amount >>>>>>>> > > of work reducing heap memory footprint of the vectors. Are target >>>>>>>> > > is to >>>>>>>> > > actually get it down to 24 bytes per ArrowBuf and 24 bytes heap >>>>>>>> > > per vector >>>>>>>> > > (not including arrow bufs). >>>>>>>> > > >>>>>>>> > > With regards to the reference implementation point. It is a good >>>>>>>> > > point. >>>>>>>> > > I'm on vacation this week. Unless you're pushing hard on this, can >>>>>>>> > > we pick >>>>>>>> > > this up and discuss more next week? >>>>>>>> > > >>>>>>>> > > thanks, >>>>>>>> > > Jacques >>>>>>>> > > >>>>>>>> > > On Sat, Aug 10, 2019 at 7:39 PM Micah Kornfield >>>>>>>> > > <emkornfi...@gmail.com> >>>>>>>> > > wrote: >>>>>>>> > > >>>>>>>> > >> Hi Jacques, >>>>>>>> > >> I definitely understand these concerns and this change is risky >>>>>>>> > >> because it >>>>>>>> > >> is so large. Perhaps, creating a new hierarchy, might be the >>>>>>>> > >> cleanest way >>>>>>>> > >> of dealing with this. This could have other benefits like >>>>>>>> > >> cleaning up >>>>>>>> > >> some >>>>>>>> > >> cruft around dictionary encode and "orphaned" method. Per past >>>>>>>> > >> e-mail >>>>>>>> > >> threads I agree it is beneficial to have 2 separate reference >>>>>>>> > >> implementations that can communicate fully, and my intent here >>>>>>>> > >> was to >>>>>>>> > >> close >>>>>>>> > >> that gap. >>>>>>>> > >> >>>>>>>> > >> Trying to >>>>>>>> > >> > determine the ramifications of these changes would be >>>>>>>> > >> > challenging and >>>>>>>> > >> time >>>>>>>> > >> > consuming against all the different ways we interact with the >>>>>>>> > >> > Arrow Java >>>>>>>> > >> > library. >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> > >> Understood. I took a quick look at Dremio-OSS it seems like it >>>>>>>> > >> has a >>>>>>>> > >> simple java build system? If it is helpful, I can try to get a >>>>>>>> > >> fork >>>>>>>> > >> running that at least compiles against this PR. My plan would be >>>>>>>> > >> to cast >>>>>>>> > >> any place that was changed to return a long back to an int, so in >>>>>>>> > >> essence >>>>>>>> > >> the Dremio algorithms would reman 32-bit implementations. >>>>>>>> > >> >>>>>>>> > >> I don't have the infrastructure to test this change properly >>>>>>>> > >> from a >>>>>>>> > >> distributed systems perspective, so it would still take some time >>>>>>>> > >> from >>>>>>>> > >> Dremio to validate for regressions. >>>>>>>> > >> >>>>>>>> > >> I'm not saying I'm against this but want to make sure we've >>>>>>>> > >> > explored all less disruptive options before considering changing >>>>>>>> > >> something >>>>>>>> > >> > this fundamental (especially when I generally hold the view >>>>>>>> > >> > that large >>>>>>>> > >> cell >>>>>>>> > >> > counts against massive contiguous memory is an anti pattern to >>>>>>>> > >> > scalable >>>>>>>> > >> > analytical processing--purely subjective of course). >>>>>>>> > >> >>>>>>>> > >> >>>>>>>> > >> I'm open to other ideas here, as well. I don't think it is out of >>>>>>>> > >> the >>>>>>>> > >> question to leave the Java implementation as 32-bit, but if we >>>>>>>> > >> do, then I >>>>>>>> > >> think we should consider a different strategy for reference >>>>>>>> > >> implementations. >>>>>>>> > >> >>>>>>>> > >> Thanks, >>>>>>>> > >> Micah >>>>>>>> > >> >>>>>>>> > >> On Sat, Aug 10, 2019 at 5:09 PM Jacques Nadeau >>>>>>>> > >> <jacq...@apache.org> >>>>>>>> > >> wrote: >>>>>>>> > >> >>>>>>>> > >> > Hey Micah, I didn't have a particular path in mind. Was >>>>>>>> > >> > thinking more >>>>>>>> > >> along >>>>>>>> > >> > the lines of extra methods as opposed to separate classes. >>>>>>>> > >> > >>>>>>>> > >> > Arrow hasn't historically been a place where we're writing >>>>>>>> > >> > algorithms in >>>>>>>> > >> > Java so the fact that they aren't there doesn't mean they don't >>>>>>>> > >> > exist. >>>>>>>> > >> We >>>>>>>> > >> > have a large amount of code that depends on the current >>>>>>>> > >> > behavior that is >>>>>>>> > >> > deployed in hundreds of customer clusters (you can peruse our >>>>>>>> > >> > dremio >>>>>>>> > >> repo >>>>>>>> > >> > to see how extensively we leverage Arrow if interested). Trying >>>>>>>> > >> > to >>>>>>>> > >> > determine the ramifications of these changes would be >>>>>>>> > >> > challenging and >>>>>>>> > >> time >>>>>>>> > >> > consuming against all the different ways we interact with the >>>>>>>> > >> > Arrow Java >>>>>>>> > >> > library. I'm not saying I'm against this but want to make sure >>>>>>>> > >> > we've >>>>>>>> > >> > explored all less disruptive options before considering changing >>>>>>>> > >> something >>>>>>>> > >> > this fundamental (especially when I generally hold the view >>>>>>>> > >> > that large >>>>>>>> > >> cell >>>>>>>> > >> > counts against massive contiguous memory is an anti pattern to >>>>>>>> > >> > scalable >>>>>>>> > >> > analytical processing--purely subjective of course). >>>>>>>> > >> > >>>>>>>> > >> > On Sat, Aug 10, 2019, 4:17 PM Micah Kornfield >>>>>>>> > >> > <emkornfi...@gmail.com> >>>>>>>> > >> > wrote: >>>>>>>> > >> > >>>>>>>> > >> > > Hi Jacques, >>>>>>>> > >> > > What avenue were you thinking for supporting both paths? I >>>>>>>> > >> > > didn't >>>>>>>> > >> want >>>>>>>> > >> > > to pursue a different class hierarchy, because I felt like >>>>>>>> > >> > > that would >>>>>>>> > >> > > effectively fork the code base, but that is potentially an >>>>>>>> > >> > > option that >>>>>>>> > >> > > would allow us to have a complete reference implementation in >>>>>>>> > >> > > Java >>>>>>>> > >> that >>>>>>>> > >> > can >>>>>>>> > >> > > fully interact with C++, without major changes to this code. >>>>>>>> > >> > > >>>>>>>> > >> > > For supporting both APIs on the same classes/interfaces, I >>>>>>>> > >> > > think they >>>>>>>> > >> > > roughly fall into three categories, changes to input >>>>>>>> > >> > > parameters, >>>>>>>> > >> changes >>>>>>>> > >> > to >>>>>>>> > >> > > output parameters and algorithm changes. >>>>>>>> > >> > > >>>>>>>> > >> > > For inputs, changing from int to long is essentially a no-op >>>>>>>> > >> > > from the >>>>>>>> > >> > > compiler perspective. From the limited micro-benchmarking >>>>>>>> > >> > > this also >>>>>>>> > >> > > doesn't seem to have a performance impact. So we could keep >>>>>>>> > >> > > two >>>>>>>> > >> versions >>>>>>>> > >> > > of the methods that only differ on inputs, but it is not >>>>>>>> > >> > > clear what >>>>>>>> > >> the >>>>>>>> > >> > > value of that would be. >>>>>>>> > >> > > >>>>>>>> > >> > > For outputs, we can't support methods "long getLength()" and >>>>>>>> > >> > > "int >>>>>>>> > >> > > getLength()" in the same class, so we would be forced into >>>>>>>> > >> > > something >>>>>>>> > >> like >>>>>>>> > >> > > "long getLength(boolean dummy)" which I think is a less >>>>>>>> > >> > > desirable. >>>>>>>> > >> > > >>>>>>>> > >> > > For algorithm changes, there did not appear to be too many >>>>>>>> > >> > > places >>>>>>>> > >> where >>>>>>>> > >> > we >>>>>>>> > >> > > actually loop over all elements (it is quite possible I missed >>>>>>>> > >> something >>>>>>>> > >> > > here), the ones that I did find I was able to mitigate >>>>>>>> > >> > > performance >>>>>>>> > >> > > penalties as noted above. Some of the current implementation >>>>>>>> > >> > > will >>>>>>>> > >> get a >>>>>>>> > >> > > lot slower for "large arrays", but we can likely fix those >>>>>>>> > >> > > later or in >>>>>>>> > >> > this >>>>>>>> > >> > > PR with a nested while loop instead of 2 for loops. >>>>>>>> > >> > > >>>>>>>> > >> > > Thanks, >>>>>>>> > >> > > Micah >>>>>>>> > >> > > >>>>>>>> > >> > > >>>>>>>> > >> > > On Saturday, August 10, 2019, Jacques Nadeau >>>>>>>> > >> > > <jacq...@apache.org> >>>>>>>> > >> wrote: >>>>>>>> > >> > > >>>>>>>> > >> > >> This is a pretty massive change to the apis. I wonder how >>>>>>>> > >> > >> nasty it >>>>>>>> > >> would >>>>>>>> > >> > >> be to just support both paths. Have you evaluated how >>>>>>>> > >> > >> complex that >>>>>>>> > >> > would be? >>>>>>>> > >> > >> >>>>>>>> > >> > >> On Wed, Aug 7, 2019 at 11:08 PM Micah Kornfield < >>>>>>>> > >> emkornfi...@gmail.com> >>>>>>>> > >> > >> wrote: >>>>>>>> > >> > >> >>>>>>>> > >> > >>> After more investigation, it looks like Float8Benchmarks at >>>>>>>> > >> > >>> least >>>>>>>> > >> on my >>>>>>>> > >> > >>> machine are within the range of noise. >>>>>>>> > >> > >>> >>>>>>>> > >> > >>> For BitVectorHelper I pushed a new commit [1], seems to >>>>>>>> > >> > >>> bring the >>>>>>>> > >> > >>> BitVectorHelper benchmarks back inline (and even with some >>>>>>>> > >> improvement >>>>>>>> > >> > >>> for >>>>>>>> > >> > >>> getNullCountBenchmark). >>>>>>>> > >> > >>> >>>>>>>> > >> > >>> Benchmark Mode Cnt >>>>>>>> > >> > >>> Score >>>>>>>> > >> > >>> Error >>>>>>>> > >> > >>> Units >>>>>>>> > >> > >>> BitVectorHelperBenchmarks.allBitsNullBenchmark avgt 5 >>>>>>>> > >> > >>> 3.821 ± >>>>>>>> > >> > >>> 0.031 >>>>>>>> > >> > >>> ns/op >>>>>>>> > >> > >>> BitVectorHelperBenchmarks.getNullCountBenchmark avgt 5 >>>>>>>> > >> > >>> 14.884 ± >>>>>>>> > >> > >>> 0.141 >>>>>>>> > >> > >>> ns/op >>>>>>>> > >> > >>> >>>>>>>> > >> > >>> I applied the same pattern to other loops that I could >>>>>>>> > >> > >>> find, and for >>>>>>>> > >> > any >>>>>>>> > >> > >>> "for (long" loop on the critical path, I broke it up into >>>>>>>> > >> > >>> two loops. >>>>>>>> > >> > the >>>>>>>> > >> > >>> first loop does iteration by integer, the second finishes >>>>>>>> > >> > >>> off for >>>>>>>> > >> any >>>>>>>> > >> > >>> long >>>>>>>> > >> > >>> values. As a side note it seems like optimization for >>>>>>>> > >> > >>> loops using >>>>>>>> > >> long >>>>>>>> > >> > >>> counters at least have a semi-recent open bug for the JVM >>>>>>>> > >> > >>> [2] >>>>>>>> > >> > >>> >>>>>>>> > >> > >>> Thanks, >>>>>>>> > >> > >>> Micah >>>>>>>> > >> > >>> >>>>>>>> > >> > >>> [1] >>>>>>>> > >> > >>> >>>>>>>> > >> > >>> >>>>>>>> > >> > >>>>>>>> > >> https://github.com/apache/arrow/pull/5020/commits/2ea2c1ae83e3baa7b9a99a6d06276d968df41797 >>>>>>>> > >> > >>> [2] https://bugs.openjdk.java.net/browse/JDK-8223051 >>>>>>>> > >> > >>> >>>>>>>> > >> > >>> On Wed, Aug 7, 2019 at 8:11 PM Micah Kornfield < >>>>>>>> > >> emkornfi...@gmail.com> >>>>>>>> > >> > >>> wrote: >>>>>>>> > >> > >>> >>>>>>>> > >> > >>> > Indeed, the BoundChecking and CheckNullForGet variables >>>>>>>> > >> > >>> > can make a >>>>>>>> > >> > big >>>>>>>> > >> > >>> > difference. I didn't initially run the benchmarks with >>>>>>>> > >> > >>> > these >>>>>>>> > >> turned >>>>>>>> > >> > on >>>>>>>> > >> > >>> > (you can see the result from above with >>>>>>>> > >> > >>> > Float8Benchmarks). Here >>>>>>>> > >> are >>>>>>>> > >> > >>> new >>>>>>>> > >> > >>> > numbers including with the flags enabled. It looks like >>>>>>>> > >> > >>> > using >>>>>>>> > >> longs >>>>>>>> > >> > >>> might >>>>>>>> > >> > >>> > be a little bit slower, I'll see what I can do to >>>>>>>> > >> > >>> > mitigate this. >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > Ravindra also volunteered to try to benchmark the changes >>>>>>>> > >> > >>> > with >>>>>>>> > >> > Dremio's >>>>>>>> > >> > >>> > code on today's sync call. >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > New >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > Benchmark Mode >>>>>>>> > >> > >>> > Cnt Score >>>>>>>> > >> > >>> Error >>>>>>>> > >> > >>> > Units >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > BitVectorHelperBenchmarks.allBitsNullBenchmark avgt 5 >>>>>>>> > >> 4.176 ± >>>>>>>> > >> > >>> 1.292 >>>>>>>> > >> > >>> > ns/op >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > BitVectorHelperBenchmarks.getNullCountBenchmark avgt 5 >>>>>>>> > >> 26.102 ± >>>>>>>> > >> > >>> 0.700 >>>>>>>> > >> > >>> > ns/op >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > Float8Benchmarks.copyFromBenchmark avgt 5 7.398 ± >>>>>>>> > >> > >>> > 0.084 >>>>>>>> > >> us/op >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > Float8Benchmarks.readWriteBenchmark avgt 5 2.711 ± >>>>>>>> > >> > >>> > 0.057 >>>>>>>> > >> us/op >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > old >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > BitVectorHelperBenchmarks.allBitsNullBenchmark avgt 5 >>>>>>>> > >> 3.828 ± >>>>>>>> > >> > >>> 0.030 >>>>>>>> > >> > >>> > ns/op >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > BitVectorHelperBenchmarks.getNullCountBenchmark avgt 5 >>>>>>>> > >> 20.611 ± >>>>>>>> > >> > >>> 0.188 >>>>>>>> > >> > >>> > ns/op >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > Float8Benchmarks.copyFromBenchmark avgt 5 6.597 ± >>>>>>>> > >> > >>> > 0.462 >>>>>>>> > >> us/op >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > Float8Benchmarks.readWriteBenchmark avgt 5 2.615 ± >>>>>>>> > >> > >>> > 0.027 >>>>>>>> > >> us/op >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> > On Wed, Aug 7, 2019 at 7:13 PM Fan Liya >>>>>>>> > >> > >>> > <liya.fa...@gmail.com> >>>>>>>> > >> > wrote: >>>>>>>> > >> > >>> > >>>>>>>> > >> > >>> >> Hi Gonzalo, >>>>>>>> > >> > >>> >> >>>>>>>> > >> > >>> >> Thanks for sharing the performance results. >>>>>>>> > >> > >>> >> I am wondering if you have turned off the flag >>>>>>>> > >> > >>> >> BoundsChecking#BOUNDS_CHECKING_ENABLED. >>>>>>>> > >> > >>> >> If not, the lower throughput should be expected. >>>>>>>> > >> > >>> >> >>>>>>>> > >> > >>> >> Best, >>>>>>>> > >> > >>> >> Liya Fan >>>>>>>> > >> > >>> >> >>>>>>>> > >> > >>> >> On Wed, Aug 7, 2019 at 10:23 PM Micah Kornfield < >>>>>>>> > >> > >>> emkornfi...@gmail.com> >>>>>>>> > >> > >>> >> wrote: >>>>>>>> > >> > >>> >> >>>>>>>> > >> > >>> >>> Hi Gonzalo, >>>>>>>> > >> > >>> >>> Thank you for the feedback. I wasn't aware of the JIT >>>>>>>> > >> > >>> implications. At >>>>>>>> > >> > >>> >>> least on the benchmark run they don't seem to have an >>>>>>>> > >> > >>> >>> impact. >>>>>>>> > >> > >>> >>> >>>>>>>> > >> > >>> >>> If there are other benchmarks that people have that can >>>>>>>> > >> validate if >>>>>>>> > >> > >>> this >>>>>>>> > >> > >>> >>> change will be problematic I would appreciate trying to >>>>>>>> > >> > >>> >>> run them >>>>>>>> > >> > >>> with the >>>>>>>> > >> > >>> >>> PR. I will try to run the ones for zeroing/popcnt >>>>>>>> > >> > >>> >>> tonight to >>>>>>>> > >> see >>>>>>>> > >> > if >>>>>>>> > >> > >>> >>> there >>>>>>>> > >> > >>> >>> is a change in those. >>>>>>>> > >> > >>> >>> >>>>>>>> > >> > >>> >>> -Micah >>>>>>>> > >> > >>> >>> >>>>>>>> > >> > >>> >>> >>>>>>>> > >> > >>> >>> >>>>>>>> > >> > >>> >>> On Wednesday, August 7, 2019, Gonzalo Ortiz Jaureguizar >>>>>>>> > >> > >>> >>> < >>>>>>>> > >> > >>> >>> golthir...@gmail.com> wrote: >>>>>>>> > >> > >>> >>> >>>>>>>> > >> > >>> >>> > I would recommend to take care with this kind of >>>>>>>> > >> > >>> >>> > changes. >>>>>>>> > >> > >>> >>> > >>>>>>>> > >> > >>> >>> > I didn't try Arrow in more than one year, but by then >>>>>>>> > >> > >>> >>> > the >>>>>>>> > >> > >>> performance >>>>>>>> > >> > >>> >>> was >>>>>>>> > >> > >>> >>> > quite bad in comparison with plain byte buffer access >>>>>>>> > >> > >>> >>> > (see >>>>>>>> > >> > >>> >>> > http://git.net/apache-arrow-development/msg02353.html >>>>>>>> > >> > >>> >>> > *) >>>>>>>> > >> > and >>>>>>>> > >> > >>> >>> > there are several optimizations that the JVM >>>>>>>> > >> > >>> >>> > (specifically, >>>>>>>> > >> C2) >>>>>>>> > >> > >>> does >>>>>>>> > >> > >>> >>> not >>>>>>>> > >> > >>> >>> > apply when dealing with int instead of longs. One of >>>>>>>> > >> > >>> >>> > the >>>>>>>> > >> > >>> >>> > most commons is the loop unrolling and vectorization. >>>>>>>> > >> > >>> >>> > >>>>>>>> > >> > >>> >>> > * It doesn't seem the best way to reference an old >>>>>>>> > >> > >>> >>> > email on >>>>>>>> > >> the >>>>>>>> > >> > >>> list, >>>>>>>> > >> > >>> >>> but >>>>>>>> > >> > >>> >>> > it is the only result shown by Google >>>>>>>> > >> > >>> >>> > >>>>>>>> > >> > >>> >>> > El mié., 7 ago. 2019 a las 11:42, Fan Liya (< >>>>>>>> > >> > liya.fa...@gmail.com >>>>>>>> > >> > >>> >) >>>>>>>> > >> > >>> >>> > escribió: >>>>>>>> > >> > >>> >>> > >>>>>>>> > >> > >>> >>> >> Hi Micah, >>>>>>>> > >> > >>> >>> >> >>>>>>>> > >> > >>> >>> >> Thanks for your effort. The performance result looks >>>>>>>> > >> > >>> >>> >> good. >>>>>>>> > >> > >>> >>> >> >>>>>>>> > >> > >>> >>> >> As you indicated, ArrowBuf will take additional 12 >>>>>>>> > >> > >>> >>> >> bytes (4 >>>>>>>> > >> > bytes >>>>>>>> > >> > >>> for >>>>>>>> > >> > >>> >>> each >>>>>>>> > >> > >>> >>> >> of length, write index, and read index). >>>>>>>> > >> > >>> >>> >> Similar overheads also exist for vectors like >>>>>>>> > >> > >>> BaseFixedWidthVector, >>>>>>>> > >> > >>> >>> >> BaseVariableWidthVector, etc. >>>>>>>> > >> > >>> >>> >> >>>>>>>> > >> > >>> >>> >> IMO, such overheads are small enough to justify the >>>>>>>> > >> > >>> >>> >> change. >>>>>>>> > >> > >>> >>> >> Let's check if there are other overheads. >>>>>>>> > >> > >>> >>> >> >>>>>>>> > >> > >>> >>> >> Best, >>>>>>>> > >> > >>> >>> >> Liya Fan >>>>>>>> > >> > >>> >>> >> >>>>>>>> > >> > >>> >>> >> On Wed, Aug 7, 2019 at 3:30 PM Micah Kornfield < >>>>>>>> > >> > >>> emkornfi...@gmail.com >>>>>>>> > >> > >>> >>> > >>>>>>>> > >> > >>> >>> >> wrote: >>>>>>>> > >> > >>> >>> >> >>>>>>>> > >> > >>> >>> >> > 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 >>>>>>>> > >> > >>> >>> >> >>> >>>>>>>> > >> > >>> >>> >> >> >>>>>>>> > >> > >>> >>> >> >>>>>>>> > >> > >>> >>> > >>>>>>>> > >> > >>> >>> >>>>>>>> > >> > >>> >> >>>>>>>> > >> > >>> >>>>>>>> > >> > >> >>>>>>>> > >> > >>>>>>>> > >> >>>>>>>> > >
