[10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of() (original) (raw)

Claes Redestad claes.redestad at oracle.com
Sat Dec 9 02:45:52 UTC 2017

On 2017-12-09 02:09, John Rose wrote:

On Dec 8, 2017, at 7:13 AM, Claes Redestad <claes.redestad at oracle.com> wrote:

http://cr.openjdk.java.net/~redestad/8193128/open.03/

+ for (int i = elements.length - 1; i > 0; i--) { There's an OBOE in this line; should be i >= 0.

I thought I tested the lastIndexOf(..) == 0 cases, but obviously only for the List12 case...

Errors like this are a risk of hand-specializing code. It's probably worth it here, but it's still a risk. For immutable lists, it would be reasonable to recode the generic algorithms to use for-loops with integer indexes and get(int) access. That will remove the unused generality of iterators, and keep only the abstraction across get(). The benefits of customized inlining (to fold up the funny specialized get methods) can probably be obtained like this: class AbsImmList { @ForceInline int indexOf(Object x) { for (int i = 0, s = size(); i < s; i++) if (x.equals(this.get(i))) return i; } } final class List12 { int indexOf(Object x) { return super.indexOf(x); } // specialize size/get } final class ListN { int indexOf(Object x) { return super.indexOf(x); } // specialize size/get } The optimization of the funny 1/2 loop for List12 should unfold into the same code you'd write by hand; if not it's a bug to fix in C2. Likewise for the loop in ListN.

I agree, but left this as is for now. If we go the route of re-specifying behavior of subList we'll want to make sure we get the NPE behavior of indexOf/lastIndexOf right, too.

— John P.S. If you are going to hand-specialize, it might be easier on the optimizer if you would add in this line as needed: final E[] elements = this.elements; // cache field Usually C2 gets the same answer, but not always, since final fields are not fully trustable, and "might" (in some hypothetical system which C2 can't rule out) change over a call to Object::equals. OTOH, generic coding as above is preferable, and we can fix C2 to hoist the @Stable elements field if it doesn't already.

Ok.

P.P.S. Why does ListN have arity 1 and arity 2 constructors?

Another left-over from my earlier experiments, removed:

http://cr.openjdk.java.net/~redestad/8193128/open.04/

/Claes

More information about the core-libs-dev mailing list