RFR: 8214712: Archive Attributes$Name.KNOWN_NAMES
Hi, initializing java.util.jar.Attributes.Name.<clinit> executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects. By archiving the resulting set of Names and initializing public constants from the archived map, we reduce time spent starting up (Name.<clinit> drops to 368 executed bytecodes) and improve the footprint sharing effect of using CDS: http://cr.openjdk.java.net/~redestad/8214712/jdk.00/ Testing: tier1-2 running Verified a 1-2.5ms startup improvement on java -jar Hello.jar - significant and stable reduction in instruction count, branches and branch misses - only adds ~1.1Kb to the dumped CDS archive Thanks! /Claes
On 03/12/2018 16:02, Claes Redestad wrote:
Hi,
initializing java.util.jar.Attributes.Name.<clinit> executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects.
By archiving the resulting set of Names and initializing public constants from the archived map, we reduce time spent starting up (Name.<clinit> drops to 368 executed bytecodes) and improve the footprint sharing effect of using CDS:
http://cr.openjdk.java.net/~redestad/8214712/jdk.00/
Testing: tier1-2 running
Verified a 1-2.5ms startup improvement on java -jar Hello.jar - significant and stable reduction in instruction count, branches and branch misses - only adds ~1.1Kb to the dumped CDS archive This looks okay to me but the changes remind me that there are several attributes name in KNOWN_NAMES are should probably be removed as they aren't defined by Java SE or the JDK.
-Alan
On 2018-12-03 17:06, Alan Bateman wrote:
On 03/12/2018 16:02, Claes Redestad wrote:
Hi,
This looks okay to me but the changes remind me that there are several attributes name in KNOWN_NAMES are should probably be removed as they aren't defined by Java SE or the JDK.
Thanks, Alan! The extra Names added to KNOWN_NAMES was my doing, and it was based on commonly used attributes in Jar file manifests scanned from a set commonly used applications as an alternative to building up a Name cache dynamically (which is frought with other perils). The chose extras gave a marginal but measurable improvement in startup to a wide array of applications as a result, and their inclusion is strictly an internal implementation detail. If you insist on having them removed I won't object, but I believe it's an harmless optimization. /Claes
On 03/12/2018 16:50, Claes Redestad wrote:
:
The extra Names added to KNOWN_NAMES was my doing, and it was based on commonly used attributes in Jar file manifests scanned from a set commonly used applications as an alternative to building up a Name cache dynamically (which is frought with other perils). The chose extras gave a marginal but measurable improvement in startup to a wide array of applications as a result, and their inclusion is strictly an internal implementation detail. If you insist on having them removed I won't object, but I believe it's an harmless optimization. There are several non-standard and tool/library specify attribute name in this list, many should not be interesting to libraries on either the class path or module path. So I think we should look at pruning that list. Ideally it would like just standard and JDK-specific attributes as they are the only attributes that the JDK knows about.
-Alan
On 2018-12-04 08:42, Alan Bateman wrote:
On 03/12/2018 16:50, Claes Redestad wrote:
:
The extra Names added to KNOWN_NAMES was my doing, and it was based on commonly used attributes in Jar file manifests scanned from a set commonly used applications as an alternative to building up a Name cache dynamically (which is frought with other perils). The chose extras gave a marginal but measurable improvement in startup to a wide array of applications as a result, and their inclusion is strictly an internal implementation detail. If you insist on having them removed I won't object, but I believe it's an harmless optimization. There are several non-standard and tool/library specify attribute name in this list, many should not be interesting to libraries on either the class path or module path. So I think we should look at pruning that list. Ideally it would like just standard and JDK-specific attributes as they are the only attributes that the JDK knows about.
These non-standard and tool/library specific names appear in the manifest of a great many jar files, for example on maven central, and including the most common non-standard manifest attributes significantly reduce allocation churn reading such manifests. Especially true for ones that were oft repeated within the same manifest, e.g.,"Name" and "SHA1-Digest" for signed entries. Was it a crucial optimization? Probably not, a few percent on some startup applications. Ideals vs trade-offs... Hard-coding a bunch of arbitrarily chosen external values isn't exactly pretty, sure, but I do think the JDK should try to optimize for typical behavior, and the fact that many (most?) libraries on maven central have OSGi attributes ("Bundle-*") in their manifests is one such typical behavior, and this optimization was very low cost and net beneficial on most. With 8214712 the overhead cost is even less (almost zero in fact). /Claes
On 12/4/18 12:32 AM, Claes Redestad wrote:
These non-standard and tool/library specific names appear in the manifest of a great many jar files, for example on maven central, and including the most common non-standard manifest attributes significantly reduce allocation churn reading such manifests. Especially true for ones that were oft repeated within the same manifest, e.g.,"Name" and "SHA1-Digest" for signed entries.
Was it a crucial optimization? Probably not, a few percent on some startup applications.
Ideals vs trade-offs... Hard-coding a bunch of arbitrarily chosen external values isn't exactly pretty, sure, but I do think the JDK should try to optimize for typical behavior, and the fact that many (most?) libraries on maven central have OSGi attributes ("Bundle-*") in their manifests is one such typical behavior, and this optimization was very low cost and net beneficial on most. With 8214712 the overhead cost is even less (almost zero in fact).
It might be worth adding a comment to say that's where these names come from. That is, they appear to occur frequently "in the wild" but otherwise have no special significance or meaning in the JDK. Also, could addName() return a Map.Entry, and then you could just initialize KNOWN_NAMES using Map.ofEntries(), passing the whole list of entries from the addName() call? It might be preferable to building an intermediate HashMap. s'marks
Hi Claes, Looks good from object graph archiving perspective. Placing the KNOWN_NAMES map in the closed archive looks safe since no additional names are added after initialization. If you also agree, maybe add a comment above the KNOWN_NAMES field declaration with those information. Thanks! Jiangli On 12/3/18 8:02 AM, Claes Redestad wrote:
Hi,
initializing java.util.jar.Attributes.Name.<clinit> executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects.
By archiving the resulting set of Names and initializing public constants from the archived map, we reduce time spent starting up (Name.<clinit> drops to 368 executed bytecodes) and improve the footprint sharing effect of using CDS:
http://cr.openjdk.java.net/~redestad/8214712/jdk.00/
Testing: tier1-2 running
Verified a 1-2.5ms startup improvement on java -jar Hello.jar - significant and stable reduction in instruction count, branches and branch misses - only adds ~1.1Kb to the dumped CDS archive
Thanks!
/Claes
Hi Claes, Meta-comment: are these Names candidates for the forthcoming compile-time evaluation of constants? Just wondering if these optimizations (and even the archiving itself) will be moot in the future? Thanks, David On 4/12/2018 2:02 am, Claes Redestad wrote:
Hi,
initializing java.util.jar.Attributes.Name.<clinit> executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects.
By archiving the resulting set of Names and initializing public constants from the archived map, we reduce time spent starting up (Name.<clinit> drops to 368 executed bytecodes) and improve the footprint sharing effect of using CDS:
http://cr.openjdk.java.net/~redestad/8214712/jdk.00/
Testing: tier1-2 running
Verified a 1-2.5ms startup improvement on java -jar Hello.jar - significant and stable reduction in instruction count, branches and branch misses - only adds ~1.1Kb to the dumped CDS archive
Thanks!
/Claes
Hi David, unless my reading of JEP 334 (and JEP 303) is completely off then custom constants (say, Attributes$Name) would effectively be turned into condys, implying a bootstrap method call for them to be constructed. So would carry similar construction overhead as the current implementation. And even if I'm missing some detail of JEP 334 that would allow these Name objects to be loaded in without any runtime BSM call overhead whatsoever, we'd still need to create the KNOWN_NAMES Map of these things. So maybe create the entire map as a constant? I'm not sure we even could describe something like a Map<String, Name> as a loadable constant, but it'd be an interesting prospect... still, I believe the creation cost would end up similar to the status quo. So: while they may share some common goals (move computation out of runtime), I think there are some things heap archiving will keep doing better. Facilitating better sharing between JVMs is another benefit that comes from more extensive heap archiving. /Claes On 2018-12-04 06:09, David Holmes wrote:i
Hi Claes,
Meta-comment: are these Names candidates for the forthcoming compile-time evaluation of constants? Just wondering if these optimizations (and even the archiving itself) will be moot in the future? vev Thanks, David
On 4/12/2018 2:02 am, Claes Redestad wrote:
Hi,
initializing java.util.jar.Attributes.Name.<clinit> executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects.
By archiving the resulting set of Names and initializing public constants from the archived map, we reduce time spent starting up (Name.<clinit> drops to 368 executed bytecodes) and improve the footprint sharing effect of using CDS:
http://cr.openjdk.java.net/~redestad/8214712/jdk.00/
Testing: tier1-2 running
Verified a 1-2.5ms startup improvement on java -jar Hello.jar - significant and stable reduction in instruction count, branches and branch misses - only adds ~1.1Kb to the dumped CDS archive
Thanks!
/Claes
Hi, this RFE was stalled due an interaction with SA that has since been resolved. As it still applies cleanly I'll consider it reviewed. I'm just going to do some sanity testing (tier1) before push. Thanks! /Claes On 2018-12-03 17:02, Claes Redestad wrote:
Hi,
initializing java.util.jar.Attributes.Name.<clinit> executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects.
By archiving the resulting set of Names and initializing public constants from the archived map, we reduce time spent starting up (Name.<clinit> drops to 368 executed bytecodes) and improve the footprint sharing effect of using CDS:
http://cr.openjdk.java.net/~redestad/8214712/jdk.00/
Testing: tier1-2 running
Verified a 1-2.5ms startup improvement on java -jar Hello.jar - significant and stable reduction in instruction count, branches and branch misses - only adds ~1.1Kb to the dumped CDS archive
Thanks!
/Claes
Looks good! Thanks, Jiangli On Thu, Mar 14, 2019 at 9:26 AM Claes Redestad <claes.redestad@oracle.com> wrote:
Hi,
this RFE was stalled due an interaction with SA that has since been resolved. As it still applies cleanly I'll consider it reviewed. I'm just going to do some sanity testing (tier1) before push.
Thanks!
/Claes
On 2018-12-03 17:02, Claes Redestad wrote:
Hi,
initializing java.util.jar.Attributes.Name.<clinit> executes ~20k bytecodes setting up and eagerly calculating case-insensitive hash codes for a slew of Name objects.
By archiving the resulting set of Names and initializing public constants from the archived map, we reduce time spent starting up (Name.<clinit> drops to 368 executed bytecodes) and improve the footprint sharing effect of using CDS:
http://cr.openjdk.java.net/~redestad/8214712/jdk.00/
Testing: tier1-2 running
Verified a 1-2.5ms startup improvement on java -jar Hello.jar - significant and stable reduction in instruction count, branches and branch misses - only adds ~1.1Kb to the dumped CDS archive
Thanks!
/Claes
On 14/03/2019 16:26, Claes Redestad wrote:
Hi,
this RFE was stalled due an interaction with SA that has since been resolved. As it still applies cleanly I'll consider it reviewed. I'm just going to do some sanity testing (tier1) before push. I think we need to rollback some of the changes that we done in JDK-6805750 so that we don't have non-standard attributes in the map. We shouldn't be hard coding names such as Ant-Version or Bnd-LastModified for example.
For the current webrev then I'm concerned it is brings back legacy attributes. The concept of "installed optional packages" was removed in Java SE 9, as was the ability for JAR packaged applets to trigger downloading of optional packages. I don't think the later was ever implemented in the JDK so surprising that we are finding JAR files with those attributes now. If we can prune that down then I think the changes will be okay. -Alan.
On 2019-03-14 18:13, Alan Bateman wrote:
On 14/03/2019 16:26, Claes Redestad wrote:
Hi,
this RFE was stalled due an interaction with SA that has since been resolved. As it still applies cleanly I'll consider it reviewed. I'm just going to do some sanity testing (tier1) before push. I think we need to rollback some of the changes that we done in JDK-6805750 so that we don't have non-standard attributes in the map. We shouldn't be hard coding names such as Ant-Version or Bnd-LastModified for example.
For the current webrev then I'm concerned it is brings back legacy attributes. The concept of "installed optional packages" was removed in Java SE 9, as was the ability for JAR packaged applets to trigger downloading of optional packages. I don't think the later was ever implemented in the JDK so surprising that we are finding JAR files with those attributes now. If we can prune that down then I think the changes will be okay.
Ok. I stumbled on some new test issues in SA with this patch, so I'll need to pause this one for a while anyhow. /Claes
Hi, On 2019-03-14 18:20, Claes Redestad wrote:
On 2019-03-14 18:13, Alan Bateman wrote:
For the current webrev then I'm concerned it is brings back legacy attributes. The concept of "installed optional packages" was removed in Java SE 9, as was the ability for JAR packaged applets to trigger downloading of optional packages. I don't think the later was ever implemented in the JDK so surprising that we are finding JAR files with those attributes now. If we can prune that down then I think the changes will be okay.
Ok. I stumbled on some new test issues in SA with this patch, so I'll need to pause this one for a while anyhow.
I have a solution to the heap dump issues out for review[1], so I've cleaned up this patch, verified the failing tests pass locally and am running both through tier1-3: http://cr.openjdk.java.net/~redestad/8214712/open.01/ Thanks! /Claes [1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-March/027455....
On 15/03/2019 13:45, Claes Redestad wrote:
I have a solution to the heap dump issues out for review[1], so I've cleaned up this patch, verified the failing tests pass locally and am running both through tier1-3:
This version looks good to me. -Alan
Hi Claes, If you have observed memory churn allocating Name objects when reading jar files, then perhaps we could do something to the logic of Attributes class so that lazily allocated Name(s) would get interned too? As a separate change of course. This looks good as is. Regards, Peter On 3/15/19 2:45 PM, Claes Redestad wrote:
Hi,
On 2019-03-14 18:20, Claes Redestad wrote:
On 2019-03-14 18:13, Alan Bateman wrote:
For the current webrev then I'm concerned it is brings back legacy attributes. The concept of "installed optional packages" was removed in Java SE 9, as was the ability for JAR packaged applets to trigger downloading of optional packages. I don't think the later was ever implemented in the JDK so surprising that we are finding JAR files with those attributes now. If we can prune that down then I think the changes will be okay.
Ok. I stumbled on some new test issues in SA with this patch, so I'll need to pause this one for a while anyhow.
I have a solution to the heap dump issues out for review[1], so I've cleaned up this patch, verified the failing tests pass locally and am running both through tier1-3:
http://cr.openjdk.java.net/~redestad/8214712/open.01/
Thanks!
/Claes
[1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-March/027455....
Hi Peter, thanks for reviewing! Interning arbitrary attribute names might have some unfortunate side- effects, but a tiny - maybe one or two element - cache might be sufficient to keep the churn low in practice. Name and SHA1-Digest are the most commonly repeated non-constant attribute, generated by the jarsigner tool. Often appearing many times in a single manifest. I also added SHA-256-Digest because that's what you'd typically get if you signed a JAR file today. /Claes On 2019-03-15 17:07, Peter Levart wrote:
Hi Claes,
If you have observed memory churn allocating Name objects when reading jar files, then perhaps we could do something to the logic of Attributes class so that lazily allocated Name(s) would get interned too?
As a separate change of course. This looks good as is.
Regards, Peter
On 3/15/19 2:45 PM, Claes Redestad wrote:
Hi,
On 2019-03-14 18:20, Claes Redestad wrote:
On 2019-03-14 18:13, Alan Bateman wrote:
For the current webrev then I'm concerned it is brings back legacy attributes. The concept of "installed optional packages" was removed in Java SE 9, as was the ability for JAR packaged applets to trigger downloading of optional packages. I don't think the later was ever implemented in the JDK so surprising that we are finding JAR files with those attributes now. If we can prune that down then I think the changes will be okay.
Ok. I stumbled on some new test issues in SA with this patch, so I'll need to pause this one for a while anyhow.
I have a solution to the heap dump issues out for review[1], so I've cleaned up this patch, verified the failing tests pass locally and am running both through tier1-3:
http://cr.openjdk.java.net/~redestad/8214712/open.01/
Thanks!
/Claes
[1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-March/027455....
On 3/15/19 5:14 PM, Claes Redestad wrote:
Hi Peter,
thanks for reviewing!
Interning arbitrary attribute names might have some unfortunate side- effects,
You mean if the number of different names would get larger and larger on a long-running JVM, the cache would grow without bounds? I noticed that the Name constructor: public Name(String name) { this.hashCode = hash(name); this.name = name.intern(); } ...interns the String name. Do interned strings have a weak cache or are they kept for the entire life of JVM? Regards, Peter
but a tiny - maybe one or two element - cache might be sufficient to keep the churn low in practice.
Name and SHA1-Digest are the most commonly repeated non-constant attribute, generated by the jarsigner tool. Often appearing many times in a single manifest. I also added SHA-256-Digest because that's what you'd typically get if you signed a JAR file today.
/Claes
On 2019-03-15 17:07, Peter Levart wrote:
Hi Claes,
If you have observed memory churn allocating Name objects when reading jar files, then perhaps we could do something to the logic of Attributes class so that lazily allocated Name(s) would get interned too?
As a separate change of course. This looks good as is.
Regards, Peter
On 3/15/19 2:45 PM, Claes Redestad wrote:
Hi,
On 2019-03-14 18:20, Claes Redestad wrote:
On 2019-03-14 18:13, Alan Bateman wrote:
For the current webrev then I'm concerned it is brings back legacy attributes. The concept of "installed optional packages" was removed in Java SE 9, as was the ability for JAR packaged applets to trigger downloading of optional packages. I don't think the later was ever implemented in the JDK so surprising that we are finding JAR files with those attributes now. If we can prune that down then I think the changes will be okay.
Ok. I stumbled on some new test issues in SA with this patch, so I'll need to pause this one for a while anyhow.
I have a solution to the heap dump issues out for review[1], so I've cleaned up this patch, verified the failing tests pass locally and am running both through tier1-3:
http://cr.openjdk.java.net/~redestad/8214712/open.01/
Thanks!
/Claes
[1] http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-March/027455....
On 2019-03-15 19:06, Peter Levart wrote:
On 3/15/19 5:14 PM, Claes Redestad wrote:
Hi Peter,
thanks for reviewing!
Interning arbitrary attribute names might have some unfortunate side- effects,
You mean if the number of different names would get larger and larger on a long-running JVM, the cache would grow without bounds?
Yes.
I noticed that the Name constructor:
public Name(String name) { this.hashCode = hash(name); this.name = name.intern(); }
...interns the String name. Do interned strings have a weak cache or are they kept for the entire life of JVM?
I think you can think of them as only weakly referenced by the VM, so yes, they will be GCd if no-one keeps them alive. /Claes
participants (7)
-
Alan Bateman
-
Claes Redestad
-
David Holmes
-
Jiangli Zhou
-
Jiangli Zhou
-
Peter Levart
-
Stuart Marks