On Mon, Nov 21, 2016 at 8:47 AM, Paul Sandoz <paul.san...@oracle.com> wrote:
>
> On 21 Nov 2016, at 07:29, Martin Buchholz <marti...@google.com> wrote:
>
> Un-OK. Sometimes when microbenchmarking, we stumble upon a code sequence
> that hotspot likes. I observed 20% improvement with the below, so
> switching to that:
>
> public boolean tryAdvance(Consumer<? super E> action) {
> Objects.requireNonNull(action);
> final Object[] es = elements;
> if (fence < 0) { fence = tail; cursor = head; } // late-binding
> final int i;
> if ((i = cursor) == fence)
> return false;
> E e = nonNullElementAt(es, i);
> cursor = inc(i, es.length);
> action.accept(e);
> return true;
> }
>
>
> Ok.
>
> I suppose i should stop being surprised at these kind of things, but would
> like to know, if you happen to, why such an improvement was gained for
> explicitly inlining the getFence call. Were you reaching some inlining
> thresholds in hotspot?
>
It's worse than that. I had to un-inline AND move the cursor increment
into the "middle" of the call to accept.
Regarding getFence() - I'm not convinced having a separate method here is
ever good for humans or machines, especially when the inlined version is
one readable line of code, whereas getFence() has a non-obvious side effect.
Regarding moving array access before cursor increment: this makes it easier
for hotspot to do the array access checks and cursor increment in parallel;
otherwise hotspot (thinks it) needs to defer array access until after
cursor is written because it might be out of bounds?!
