cassandra-pr mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From belliottsmith <...@git.apache.org>
Subject [GitHub] cassandra pull request #271: 14726
Date Thu, 27 Sep 2018 15:51:47 GMT
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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Mime
View raw message