> On June 12, 2014, 11:03 p.m., Mike Drob wrote: > > Can you add a unit test to ensure correctness? Something as simple as > > sorting from two iterators that have a couple values each. Also checking > > empty values.
Thank you for the review and sorry for the delay. As for unit tests, I second what kturner mentioned but I'll make a sanity pass through the MultiIteratorTest to see if there are some interesting cases that could be added. > On June 12, 2014, 11:03 p.m., Mike Drob wrote: > > core/src/main/java/org/apache/accumulo/core/iterators/system/HeapIterator.java, > > line 28 > > <https://reviews.apache.org/r/22537/diff/1/?file=608354#file608354line28> > > > > Can we switch this to be a java.util.PriorityQueue<E>? It will save us > > on unsafe casts when we pop elements. This is a good suggestion. The unsafe casts also bother me. I'll work towards getting that in. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22537/#review45550 ----------------------------------------------------------- On June 12, 2014, 9:54 p.m., Jonathan Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22537/ > ----------------------------------------------------------- > > (Updated June 12, 2014, 9:54 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2827 > https://issues.apache.org/jira/browse/ACCUMULO-2827 > > > Repository: accumulo > > > Description > ------- > > HeapIterator optimization to avoid always re-inserting back into the heap. > > > Diffs > ----- > > > core/src/main/java/org/apache/accumulo/core/iterators/system/HeapIterator.java > e54f37c > > core/src/test/java/org/apache/accumulo/core/iterators/system/BenchmarkMultiIterator.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/22537/diff/ > > > Testing > ------- > > mvn clean install > > accumulo continuous ingest: saw ~5% improvement on uniformly distributed > data. (see ticket for more details) > > > Thanks, > > Jonathan Park > >
