My remarks: - NativeArray instances no longer need PushLinkLogic, PopLinkLogic, and ConcatLinkLogic objects. These objects are no longer instance-specific (their state is only in LinkLogic.switchpoints, but none of the ArrayLinkLogic subclasses override getOrCreateModificationSwitchpoint, so it's always null. You could just have NativeArray.getLinkLogic return static instances of these three that'd eliminate three reference fields from every array instance (similar to how CharCodeLinkingLogic only has one instance). - Related: LinkLogic switch points are no longer used, the logic for handling them should be removed (or moved into a patch until needed again). - LengthNotWritableFilter(ArrayData, TreeMap) construtor shouldn't be copying the map; its invokers should ensure that data structures are copied when needed. - LengthNotWritableFilter.sliceMap(from, to) is not needed, it should be replaced with simply new TreeMap(extraElements.subMap(from, to)). - LengthNotWritableFilter.indexIterator() could just use "final List<Long> keys = computeIteratorKeys();" as the first line. The construction guarantees that the elements will be stricty monotonously ordered even after adding in the extraElements keySet. If needed, an "assert isStrictlyMonotonous(keys)" could be added, writing a simple linear function "boolean isStrictlyMonotonous(List<Long>)".
On Nov 12, 2014, at 11:41 AM, Hannes Wallnoefer <[email protected]> wrote: > A few minor remarks: > > - Shouldn't ArrayData.increaseLength and .decreaseLength be protected instead > of public? > - Javadoc of ArrayData.length is missing # for link: {@link #setLength} > (although it's a private field anyway...) > - Is the (index >= len) check needed in NativeArray.sort? Does not > ArrayData.indexIterator take care of this? > - Some of the new tests use a mix of spaces/tabs for indentation. > > Otherwise looks good. > > Hannes > > Am 2014-11-11 um 17:37 schrieb Marcus Lagergren: >> Please review >> >> https://bugs.openjdk.java.net/browse/JDK-8035312 >> <https://bugs.openjdk.java.net/browse/JDK-8035312> >> >> There were several corner cases related to length in general and setting >> length in arrays to not writable in particular. >> None of the existing run times pass all the tests, so this was a very hard >> area to get right (added 6 new unit tests) >> >> I’ve also gotten rid of the special casey length not writable SwitchPoint in >> NativeArray - now that I have a filter for LengtNotWritableArray that can’t >> be cast to a ContinuousArrayData in the fast paths, this handles itself >> anyway. >> >> webrev at: http://cr.openjdk.java.net/~lagergren/8035312/ >> >> /M >> >
