On Sep 5, 2014, at 1:25 PM, Vladimir Ivanov <vladimir.x.iva...@oracle.com> wrote:
> 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. > +1 on review. >> >> 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. > Ok. >> 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%) > OK, good to see some numbers on this, quite reassuring. Paul.