Avoid list copy on reverse iteration

[gregw]

The changes in #12289 introduced several list copy and reverses operations on every request.
This PR replaces that reversed list with a ListIterator used with previous

[gregw]

@sbordet @lorban an alternative approach would be to have the following method:

    /**
     * <p>Returns an {@link Iterator} that efficiently iterates the passed list in reverse.</p>
     * <p>The method is safe for concurrent modifications iff the list iterator itself is safe.</p>
     * @param list the list
     * @param <T> the element type
     * @return An {@link Iterator} over the list elements in reverse order
     * at the snapshot of the list represented by this call.
     */
    public static <T> Iterator<T> reversed(List<T> list)
    {
        ListIterator<T> i;
        try
        {
            int size = list.size();
            switch (size)
            {
                case 0:
                    return Collections.emptyIterator();
                case 1:
                    return list.iterator();
                default:
                    i = list.listIterator(size);
            }
        }
        catch (IndexOutOfBoundsException e)
        {
            // list was concurrently modified, so do this the hard way
            i = list.listIterator();
            while (i.hasNext())
                i.next();
        }

        ListIterator<T> iterator = i;
        return new Iterator<T>()
        {
            @Override
            public boolean hasNext()
            {
                return iterator.hasPrevious();
            }

            @Override
            public T next()
            {
                return iterator.previous();
            }
        };
    }

[sbordet]

Just javadocs niggles.

[lorban]

Instead of modifying the iterations, we could keep TypeUtil.reverse() and implement it this way:

    public static <T> Iterable<T> reverse(List<T> list)
    {
        return () -> new Iterator<>()
        {
            private final ListIterator<T> listIterator = listIteratorAtEnd(list);

            @Override
            public boolean hasNext()
            {
                return listIterator.hasPrevious();
            }

            @Override
            public T next()
            {
                return listIterator.previous();
            }

            @Override
            public void remove()
            {
                listIterator.remove();
            }
        };
    }

[sbordet]

@lorban but that's another allocation.

[lorban]

@sbordet possibly, but I sometimes feel we're a bit too obsessed with allocations so I'm tempted to spend some time looking at the JIT's efficiency at generating stack allocating code to deny/confirm if our fears are justified.

In all case, I think exposing an iterator that delegates to ListIterator's previous() and hasPrevious() like @gregw suggested in his alternative would still look better IMHO.

[joakime]

Please fix the title of this PR, so it makes sense on a changelog.

[gregw]

Instead of modifying the iterations, we could keep TypeUtil.reverse() and implement it this way:

The resulting list would have an list with an iterator that does not match the get(int) behaviour. If that list ever got out into the wild, then chaos would reign.

However, I'm not totally opposed to my alternative, even though that is an extra allocation.... The resulting code is a little neater.... but then it pretty simple code anyway, and seldom touched, and clear what it does, so in this case the extra allocation doesn't get us much.

[lorban]

Yes, this could be improved, but I agree that this code is simple enough that no improvement would bring very much value; so LGTM the way it is.