Paul, thanks for review.

Generally looks good (re: Peter's observation of continue/break).

- LambdaFormEditor

   61     private static final class Transform {
   62         final long packedBytes;
   63         final byte[] fullBytes;
   64         final LambdaForm result;  // result of transform, or null, if 
there is none available
   65
   66         private enum Kind {

More an oddity really, Transform.Kind is private but exposed via package 
private methods.
Good point. I'll fix that.


  187         static Transform of(Kind k, int b1, byte[] b234) {


It might be worthwhile investing in a unit test for LambdaFormEditor to 
exercise the transform types and transitions, otherwise edge cases might bite 
us later.
Tests for JEP 210 (SQE is working on them) should cover that.

I was thinking Transform could be greatly simplified if one just uses byte[], 
but IIUC (via JOL) that increases the instance size by 16 bytes, although that 
may be small compared to the LambdaForm result and if say a WekRef is also used 
to hold that result. Plus if that is the case the transformCache could be 
simplified by just supporting Transform[] and ConcurrentHashMap. I guess the 
underlying question i am asking here is do these space savings really give us 
much over some less complicated code?
All these specializations can be considered overoptimizations, but I'd prefer to leave them. Complexity is manageable, encapsulated, and localized ([1],[2]).

And these specializations are for the common case. The numbers I got for Octane shows that:
  (1) large transforms are very rare:
        98-99% of Transforms fit into packed representation;
  (2) for LF caches
      (a) 70-80% are single-element;
      (b) 20-30% fit into array (<16 elements)
      (c) CHM is needed very rarely (<<1%)

Best regards,
Vladimir Ivanov

[1] http://cr.openjdk.java.net/~vlivanov/lfc/editor.no_packed
[2] http://cr.openjdk.java.net/~vlivanov/lfc/editor.no_single_cache

Reply via email to