Refactor getJarMemberSet(...) & getResourceFromLocation(...)

[DanskerDave]

Bugfix for missing Jar contents mitigation & 33x performance improvement.
See: https://groups.google.com/forum/#!topic/project-lombok/YNcYxVPM8as

[rspilker]

We've updated the style back to the usual project style (not to get into a style fight at all, but consistency is probably a good thing. I hope you don't mind!), and made one very minor update to speed up some bit calculation (removed the while loop, made it a mathy thing).

[DanskerDave]

Hi Roel,

regarding the while loop: I "borrowed" that from HashMap in Java 6.
Interestingly enough, in Java 8 they have replaced the while with the
following:
static final int tableSizeFor(int cap) {
int n = cap - 1;
n |= n >>> 1;
n |= n >>> 2;
n |= n >>> 4;
n |= n >>> 8;
n |= n >>> 16;
return (n < 0) ? 1 : (n >= MAXIMUM_CAPACITY) ? MAXIMUM_CAPACITY
: n + 1;
}

Quite clever. (But I also like your solution too)
That bit of code is only ever performed once (or very rarely) though... :-)

What is less good, is that "final" has been consistently removed.
I have seen significant performance improvements with Java 7 just from
using final.
I have also spoken to "experts" who say that is impossible, but they
were measurable, consistent & repeatable.

But much more important: "final" is a statement that this variable is
not going to change.
That is an important aid to understanding a program and contribution to
"clean code".
(which in my opinion project lombok is too, although many purists
think its bad medicine to have all that going on under the hood)

Anyway, I'm proud to be able to contribute to lombok.

All the best,
DaveLaw

On 26/05/2015 22:45, Roel Spilker wrote:

We've updated the style back to the usual project style (not to get
into a style fight at all, but consistency is probably a good thing. I
hope you don't mind!), and made one very minor update to speed up some
bit calculation (removed the while loop, made it a mathy thing).

—
Reply to this email directly or view it on GitHub
#68 (comment).

[DanskerDave]

Hi again Roel,

I just wanted to touch base with you about something I've been mulling
over...

I didn't make any change to the memory-footprint of ShadowClassLoader.
It still loads the complete path for each member & that has been
bothering me!

I've tried about 5 different implementations to emulate the jar
directory structure, for example:

• recursively nested TreeMaps
• a pure 2D array-based Hash
• or just a lookaside array for the directory name

The result is, I've come to the conclusion "how much memory does it use?"
is quite a philosophical question in the Java OO world!
Whats the use of me splitting a Path into its component substrings, if
somewhere else its being stored as 1 String?!
So you really do need to know your application inside out.
And its difficult to measure anyway:
the eclipse Memory Analysis plugin lies blatantly about the retained
heapsize of HashMaps.
It tells me the entire size of all paths in the lombok jar is 27,792 bytes.
But the combined length of the 802 pathnames in my jar is 39,313 chars
& java is massively wasteful with memory (even more so in 64-bit),
so with UTF-16, 4 times that is more realistic.

I know, who cares!

Anyway, my best performer, a LinkedHashMap<String, LinkedHashSet>
is 25% slower on retrieval & uses a little less memory. I think! But
I'm not really sure... :-)

The fastest solution is the one you've just checked in.

Anyway, the ravings of a madman, but I just wanted to share that with
someone!!

All the best,
DaveLaw

On 27/05/2015 00:07, David Law wrote:

Hi Roel,

regarding the while loop: I "borrowed" that from HashMap in Java 6.
Interestingly enough, in Java 8 they have replaced the while with the
following:
static final int tableSizeFor(int cap) {
int n = cap - 1;
n |= n >>> 1;
n |= n >>> 2;
n |= n >>> 4;
n |= n >>> 8;
n |= n >>> 16;
return (n < 0) ? 1 : (n >= MAXIMUM_CAPACITY) ?
MAXIMUM_CAPACITY : n + 1;
}

Quite clever. (But I also like your solution too)
That bit of code is only ever performed once (or very rarely)
though... :-)

What is less good, is that "final" has been consistently removed.
I have seen significant performance improvements with Java 7 just from
using final.
I have also spoken to "experts" who say that is impossible, but they
were measurable, consistent & repeatable.

But much more important: "final" is a statement that this variable is
not going to change.
That is an important aid to understanding a program and contribution
to "clean code".
(which in my opinion project lombok is too, although many purists
think its bad medicine to have all that going on under the hood)

Anyway, I'm proud to be able to contribute to lombok.

All the best,
DaveLaw

On 26/05/2015 22:45, Roel Spilker wrote:

We've updated the style back to the usual project style (not to get
into a style fight at all, but consistency is probably a good thing.
I hope you don't mind!), and made one very minor update to speed up
some bit calculation (removed the while loop, made it a mathy thing).

—
Reply to this email directly or view it on GitHub
#68 (comment).

[rspilker]

Lombok certainly isn't even remotely close to the 'cleanest' code that I think I'm capable of writing; there's a lot of leeway given to the fact that lombok is a bit hacky in nature.

'final' local variables produce the exact same bytecode. It is simply impossible for it to make any performance difference.

final fields on the other hand, that's a different story.

Where there is a tossup between slightly more 'explicit' code such as 'final', and less code, we tend to lean towards the 'less code' side of the spectrum. This applies in particular to final variables and parameters. We do still like the notion of declaring variables as unchangable, which is why 'lombok.val' is a thing, of course!

We thought WE took performance tweaking seriously, but clearly we have a lot to learn here. You've taken it to awesome extremes.

Thanks for the push!

-Roel and Reinier

[DanskerDave]

Hi Roel and Reinier,

yeah, for old mainframers performance has always been a virtue.

You young guys just buy a faster box. :-)

Anyway, I ran your tweaked power-of-2 calculation through my benchmark
harness.

As you can see, the Java guys managed to improve a bit with the Java 8
changes to HashMap.
But your elegant little algorithm (very similar to Java 8 HashMap) just
beat them. By a whisker!

Start time.: 23:05:13.106
End time...: 00:47:47.862
Iterations.: 1,073,741,824
PowerRoel2.: 0.574426 µs (Factor.: 1.000) Roels tweak with line 1 :
jarSize==0 ? return 1
PowerRoel..: 0.577829 µs (Factor.: 1.006) Roels tweak
PowerJSE8..: 0.577894 µs (Factor.: 1.006) JSE 8 HashMap (Roels tweak
inlined, but 2 int calcs!)
PowerJSE6..: 0.581973 µs (Factor.: 1.013) JSE 6 HashMap

(PowerRoel2 only works in my benchmark harness, as the tests are wrapped
in method-calls,
so as line 1 you can do: if (jarSize == 0) {return 1;} )

I wonder...
...whether lombok could be used to get an even more finegrained benchmark!

@benchmark
...
@Benchover(longVarname)

All the best,
DaveLaw

On 27/05/2015 01:55, Roel Spilker wrote:

Lombok certainly isn't even remotely close to the 'cleanest' code that
I think I'm capable of writing; there's a lot of leeway given to the
fact that lombok is a bit hacky in nature.

'final' local variables produce the exact same bytecode. It is simply
impossible for it to make any performance difference.

final fields on the other hand, that's a different story.

Where there is a tossup between slightly more 'explicit' code such as
'final', and less code, we tend to lean towards the 'less code' side
of the spectrum. This applies in particular to final variables and
parameters. We do still like the notion of declaring variables as
unchangable, which is why 'lombok.val' is a thing, of course!

We thought /WE/ took performance tweaking seriously, but clearly we
have a lot to learn here. You've taken it to awesome extremes.

Thanks for the push!

-Roel and Reinier

—
Reply to this email directly or view it on GitHub
#68 (comment).