The algorithmic change (string cache) is acceptable, although it will tend to increase footprint somewhat. We can tweak that later if necessary. There are probably only a small number of such strings, so maybe a small leaky map would do the trick as well. What you propose is fine.
But, please don't hand-inline methods (checkPtype, checkSlotCount). That is usually the wrong answer. If there is a JIT inlining decision that isn't going right, we need to fix that, not distort the Java code. Also, I think you deleted a call to checkSlotCount that should still be there to detect illegal inputs. One of the costs of hand-inlining is bitrot like that. — John On Sep 11, 2013, at 12:03 PM, Sergey Kuksenko <sergey.kukse...@oracle.com> wrote: > On 09/11/2013 09:01 PM, Aleksey Shipilev wrote: >> On 09/11/2013 08:23 PM, Sergey Kuksenko wrote: >>> http://cr.openjdk.java.net/~skuksenko/jsr335/8024635/webrev.00/ >> >> As much as I hate to see the hand code tweaking instead of relying on >> compiler to do it's job, I understand this is about interpreter. Seems >> good then. >> >> * Formatting: "if(...)" should be "if (...") > Will do >> >> * Formatting: "//NPE" should be "// null check" > I just preserve exiting comments. > Decided don't modify original code which is not important to required > functionality. > >> >> * Formatting: "desc = " should be "desc = " >> >> * Formatting: this one should not use braces (for consistency with other >> usages)? >> 364 if(nptype == null) { //NPE >> 365 throw new NullPointerException(); >> 366 } > Let ask code owner. >> >> * Explicit null-checks: implicits via .getClass and .equals always >> bothered me in j.l.i.*; the idea was seemingly to piggyback on the >> compiler intrinsics. > anyway it is doesn't matter after C1/C2 > >> Any idea what's the cost of using >> Objects.requireNonNull there? > If we are running under the interpreter Objects.requireNonNull costs > enough to eliminate benefits from this fine tuning. > > -- > Best regards, > Sergey Kuksenko
