[jdk17u-dev] RFR: 8304976: Optimize DateTimeFormatterBuilder.ZoneTextPrinterParser.getTree()

[Oli Gillespie]

On Thu, 1 Jun 2023 09:29:36 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> You can still change `HashMap.newHashMap(1)` in original backport to `new HashMap(1)`, instead of dropping them. Or am I missing something here?

Yes that's true, but it doesn't seem particularly valuable, and discussion on the details might easily outweigh any benefits. There wasn't much evidence provided for the original change, anyway.

For example, it should be `2` to match upstream behaviour due to the load factor. Using `1` appears to be slower (though lower alloc) than the default.


 @Benchmark
 public HashMap init0() {
     HashMap<String, String> x = new HashMap<>();
     x.put("1", "2");
     return x;
 }
 @Benchmark
 public HashMap init1() {
     HashMap<String, String> x = new HashMap<>(1);
     x.put("1", "2");
     return x;
 }
 @Benchmark
 public HashMap init2() {
     HashMap<String, String> x = new HashMap<>(2);
     x.put("1", "2");
     return x;
 }



Benchmark                              Mode  Cnt     Score     Error   Units
HashMapInit.init0                      avgt   20    24.655 ?   0.163   ns/op
HashMapInit.init0:?gc.alloc.rate       avgt   20  6187.811 ?  40.647  MB/sec
HashMapInit.init0:?gc.alloc.rate.norm  avgt   20   160.002 ?   0.006    B/op
HashMapInit.init0:?gc.count            avgt   20   136.000            counts
HashMapInit.init0:?gc.time             avgt   20   101.000                ms
HashMapInit.init1                      avgt   20    33.716 ?   0.492   ns/op
HashMapInit.init1:?gc.alloc.rate       avgt   20  3620.473 ?  51.771  MB/sec
HashMapInit.init1:?gc.alloc.rate.norm  avgt   20   128.000 ?   0.001    B/op
HashMapInit.init1:?gc.count            avgt   20   123.000            counts
HashMapInit.init1:?gc.time             avgt   20    86.000                ms
HashMapInit.init2                      avgt   20    22.776 ?   0.877   ns/op
HashMapInit.init2:?gc.alloc.rate       avgt   20  4361.898 ? 171.144  MB/sec
HashMapInit.init2:?gc.alloc.rate.norm  avgt   20   104.001 ?   0.004    B/op
HashMapInit.init2:?gc.count            avgt   20   116.000            counts
HashMapInit.init2:?gc.time             avgt   20    84.000                ms


I don't mind changing it if it's preferred, but I think the benefit is questionable so I lean toward the simpler change.

-------------

PR Comment: https://git.openjdk.org/jdk17u-dev/pull/1414#issuecomment-1571841405