Github user belliottsmith commented on a diff in the pull request:

    https://github.com/apache/cassandra/pull/271#discussion_r220979153
  
    --- Diff: 
src/java/org/apache/cassandra/locator/AbstractReplicaCollection.java ---
    @@ -58,45 +63,332 @@
             };
         }
     
    -    protected final List<Replica> list;
    -    protected final boolean isSnapshot;
    -    protected AbstractReplicaCollection(List<Replica> list, boolean 
isSnapshot)
    +    /**
    +     * A simple list with no comodification checks and immutability by 
default (only append permitted, and only one initial copy)
    +     * this permits us to reduce the amount of garbage generated, by not 
wrapping iterators or unnecessarily copying
    +     * and reduces the amount of indirection necessary, as well as 
ensuring monomorphic callsites
    +     */
    +    protected static class ReplicaList implements Iterable<Replica>
         {
    -        this.list = list;
    -        this.isSnapshot = isSnapshot;
    +        private static final Replica[] EMPTY = new Replica[0];
    +        Replica[] contents;
    +        int begin, size;
    +
    +        public ReplicaList() { this(0); }
    +        public ReplicaList(int capacity) { contents = capacity == 0 ? 
EMPTY : new Replica[capacity]; }
    +        public ReplicaList(Replica[] contents, int begin, int size) { 
this.contents = contents; this.begin = begin; this.size = size; }
    +
    +        public boolean isSubList(ReplicaList subList)
    +        {
    +            return subList.contents == contents;
    +        }
    +
    +        public Replica get(int index)
    +        {
    +            if (index > size)
    +                throw new IndexOutOfBoundsException();
    +            return contents[begin + index];
    +        }
    +
    +        public void add(Replica replica)
    +        {
    +            // can only add to full array - if we have sliced it, we must 
be a snapshot
    +            assert begin == 0;
    +            if (size == contents.length)
    +            {
    +                int newSize;
    +                if (size < 3) newSize = 3;
    +                else if (size < 9) newSize = 9;
    +                else newSize = size * 2;
    +                contents = Arrays.copyOf(contents, newSize);
    +            }
    +            contents[size++] = replica;
    +        }
    +
    +        public int size()
    +        {
    +            return size;
    +        }
    +
    +        public boolean isEmpty()
    +        {
    +            return size == 0;
    +        }
    +
    +        public ReplicaList subList(int begin, int end)
    +        {
    +            if (end > size || begin > end) throw new 
IndexOutOfBoundsException();
    +            return new ReplicaList(contents, this.begin + begin, end - 
begin);
    +        }
    +
    +        public ReplicaList sorted(Comparator<Replica> comparator)
    +        {
    +            Replica[] copy = Arrays.copyOfRange(contents, begin, begin + 
size);
    +            Arrays.sort(copy, comparator);
    +            return new ReplicaList(copy, 0, copy.length);
    +        }
    +
    +        public Stream<Replica> stream()
    +        {
    +            return Arrays.stream(contents, begin, begin + size);
    +        }
    +
    +        @Override
    +        public Iterator<Replica> iterator()
    +        {
    +            return new Iterator<Replica>()
    +            {
    +                final int end = begin + size;
    +                int i = begin;
    +                @Override
    +                public boolean hasNext()
    +                {
    +                    return i < end;
    +                }
    +
    +                @Override
    +                public Replica next()
    +                {
    +                    return contents[i++];
    +                }
    +            };
    +        }
    +
    +        public <K> Iterator<K> transformIterator(Function<Replica, K> 
function)
    +        {
    +            return new Iterator<K>()
    +            {
    +                final int end = begin + size;
    +                int i = begin;
    +                @Override
    +                public boolean hasNext()
    +                {
    +                    return i < end;
    +                }
    +
    +                @Override
    +                public K next()
    +                {
    +                    return function.apply(contents[i++]);
    +                }
    +            };
    +        }
    +
    +        private Iterator<Replica> filterIterator(Predicate<Replica> 
predicate, int limit)
    +        {
    +            return new Iterator<Replica>()
    --- End diff --
    
    I deliberately chose not to, given how dreadfully simple they are.  Given 
that the call sites will also (generally) be monomorphic, we can guarantee they 
will be extremely cheap to evaluate, roughly equal to a for-loop (although, 
regrettably, cannot absolutely eliminate any allocations).  The same cannot 
*probably* be said for the Guava implementations, although I would have to take 
a look at the emitted ASM to be sure.
    
    That said, it's absolutely up for debate.  I'm not at all wed to the idea, 
and if you feel strongly we can drop it.  I don't think Guava is necessarily a 
great default of behaviour for very much, but also some people might dislike 
the idea of having a simple utility implemented like this here.  We could of 
course instead introduce our own array iteration utilities for these three 
simple scenario, but then we have to think of something to name it.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org

Reply via email to