Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On 2/11/22 19:25, XenoAmess wrote: On Fri, 11 Feb 2022 18:24:49 GMT, Andrew Haley wrote: Just multiply by 0.75. On a modern design, floating-point multiply is 4 clocks latency, 4 ops/clock throughput. FP max is 2 clocks latency, conversions int-float and vice versa 3 clocks latency, 4 ops/clock throughput. Long division is 7-9 clocks, 2ops/clock throughput. Shift and add 2 clocks, 2/3 ops/clock througput. Compare is 1 clock, 3 ops/clock throughput, conditional move is 1 clock, 3 ops/clock throughput. Seems like it's a wash. @theRealAph no multiply but divide. Well yes, but that doesn't look at all hard to change. besides, did you count the cost for Math.ceil? it is the heaviest part. Yes. 3 clocks latency, 4 ops/clock throughput. Your hardware may vary. And that instruction does both the ceil() and the float-int conversion. (Having said that, I don't know if we currently generate optimal code for this operation. But of course that can be fixed.) I don't think this is terribly important, but I don't like to see attempts at hand optimization in the standard library.
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v7]
> 8281631: HashMap.putAll can cause redundant space waste XenoAmess has updated the pull request incrementally with one additional commit since the last revision: 9072610: HashMap.putAll can cause redundant space waste - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/4e42cae1..95dc4c1b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=05-06 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v6]
On Tue, 15 Feb 2022 03:45:19 GMT, Stuart Marks wrote: > > A test will fail if not change codes there. Every pr should pass ci, so I > > have no choice. > > Hm, yes I recall in the preliminary email that there was some mention of a > test. However, the test seemed to use the same (incorrect) calculation, so > maybe the test needs to be fixed instead. These changes in those 2 class already the minimal changes for passing ci, as that test itself seems meaningful so I don't wanna shut it down. > Offhand, the HashMap/LinkedHashMap and the corresponding Set classes, and > WeakHashMap, are the main places to look. IdentityHashMap and the Map.of() > implementations use a different organization so are probably unrelated. > ConcurrentHashMap is another obvious place; you might want to investigate > there, but depending on the fix (if any) we might want to handle it > separately. I'd search for "loadFactor" or "LOAD_FACTOR" and see if anything > else turns up. Thanks, I will start from there and see if can found something interesting. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v6]
On Tue, 15 Feb 2022 02:12:48 GMT, XenoAmess wrote: > A test will fail if not change codes there. Every pr should pass ci, so I > have no choice. Hm, yes I recall in the preliminary email that there was some mention of a test. However, the test seemed to use the same (incorrect) calculation, so maybe the test needs to be fixed instead. > Good question. Can I get a list of classes where I should check？（I guesd I > shall start at LinkedHashMap and hash sets， but have no further ideas） Offhand, the HashMap/LinkedHashMap and the corresponding Set classes, and WeakHashMap, are the main places to look. IdentityHashMap and the Map.of() implementations use a different organization so are probably unrelated. ConcurrentHashMap is another obvious place; you might want to investigate there, but depending on the fix (if any) we might want to handle it separately. I'd search for "loadFactor" or "LOAD_FACTOR" and see if anything else turns up. > OK I will have a try... a hard part is how to read private field in class but > I think I can find some clue in the existed tests. There is an incantation in the test header to ensure that the java.base module is opened for reflection. Let me know if you have trouble with it. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v6]
On Tue, 15 Feb 2022 02:00:35 GMT, Stuart Marks wrote: > The changes to j.l.Class and the EnumConstantDirectory test don't belong here > -- these are _uses_ of HashMap. This bug and fix should focus on HashMap > itself, to ensure that the cases in question allocate a table of the right > size. A test will fail if not change codes there. > Are there any other maps that have this computation besides HashMap and > WeakHashMap? good question. Can I get a list of classes where I should check？（I guesd I shall start at linkerhashmap and sets， but have no further ideas） > There should be a regression test for this. It's probably sufficient to base > this on your original test program, which puts 12 entries into a HashMap > using a variety of techniques. It should assert that the table size is 16 in > all cases. Also, should there be a test case for WeakHashMap? OK I will havr a try... > Also, I changed the summary of the bug report to be more precise. The PR > title will need to be changed to correspond to it. Thanks. OK. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v6]
On Fri, 11 Feb 2022 19:32:48 GMT, XenoAmess wrote: >> 8281631: HashMap.putAll can cause redundant space waste > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > 9072610: HashMap.putAll can cause redundant space waste OK. The changes to HashMap and WeakHashMap look like they're on the right track. The changes to j.l.Class and the EnumConstantDirectory test don't belong here -- these are _uses_ of HashMap. This bug and fix should focus on HashMap itself, to ensure that the cases in question allocate a table of the right size. Are there any other maps that have this computation besides HashMap and WeakHashMap? There should be a regression test for this. It's probably sufficient to base this on your original test program, which puts 12 entries into a HashMap using a variety of techniques. It should assert that the table size is 16 in all cases. Also, should there be a test case for WeakHashMap? Also, I changed the summary of the bug report to be more precise. The PR title will need to be changed to correspond to it. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v6]
> 8281631: HashMap.putAll can cause redundant space waste XenoAmess has updated the pull request incrementally with one additional commit since the last revision: 9072610: HashMap.putAll can cause redundant space waste - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/c6654478..4e42cae1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=04-05 Stats: 3 lines in 2 files changed: 0 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste
On Fri, 11 Feb 2022 18:40:53 GMT, Stuart Marks wrote: > Let's stick with the `Math.ceil` expression please. I'm afraid Math.ceil is much too time costing, but fine if you want. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On Fri, 11 Feb 2022 18:24:49 GMT, Andrew Haley wrote: > Just multiply by 0.75. > > On a modern design, floating-point multiply is 4 clocks latency, 4 ops/clock > throughput. FP max is 2 clocks latency, conversions int-float and vice versa > 3 clocks latency, 4 ops/clock throughput. Long division is 7-9 clocks, > 2ops/clock throughput. Shift and add 2 clocks, 2/3 ops/clock througput. > Compare is 1 clock, 3 ops/clock throughput, conditional move is 1 clock, 3 > ops/clock throughput. > > Seems like it's a wash. @theRealAph no multiply but divide. besides, did you count the cost for Math.ceil? it is the heaviest part. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v5]
On Fri, 11 Feb 2022 17:10:43 GMT, XenoAmess wrote:
>> 8281631: HashMap.putAll can cause redundant space waste
>
> XenoAmess has updated the pull request incrementally with one additional
> commit since the last revision:
>
> 9072610: HashMap.putAll can cause redundant space waste
> > FWIW, (int) Math.ceil(expected / 0.75) and (int) ((expected * 4L + 2L) /
> > 3L) would be equivalent.
>
> No, they are not equivalent. If `expected` exceeds a certain value around
> 1.6bn, then the intermediate result using the second expression will be
> greater than Integer.MAX_VALUE. Casting this to `int` can result in a
> negative number.
> > FWIW, (int) Math.ceil(expected / 0.75) and (int) ((expected * 4L + 2L) /
> > 3L) would be equivalent.
that is exactly why we added this check when it can reach such situation.
if (size > (int) (Integer.MAX_VALUE * 0.75)) {
return Integer.MAX_VALUE;
}
-
PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v5]
On Fri, 11 Feb 2022 18:42:11 GMT, Stuart Marks wrote: > (It's too late now, but I'd suggest avoiding force-pushing changes into a > branch that's opened in a PR. Doing so confuses the comment history, as the > comments refer to code that is no longer present.) got it. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v5]
On Fri, 11 Feb 2022 17:10:43 GMT, XenoAmess wrote: >> 8281631: HashMap.putAll can cause redundant space waste > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > 9072610: HashMap.putAll can cause redundant space waste (It's too late now, but I'd suggest avoiding force-pushing changes into a branch that's opened in a PR. Doing so confuses the comment history, as the comments refer to code that is no longer present.) - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste
On Fri, 11 Feb 2022 12:25:08 GMT, XenoAmess wrote: > FWIW, (int) Math.ceil(expected / 0.75) and (int) ((expected * 4L + 2L) / 3L) > would be equivalent. No, they are not equivalent. If `expected` exceeds a certain value around 1.6bn, then the intermediate result using the second expression will be greater than Integer.MAX_VALUE. Casting this to `int` can result in a negative number. In the first expression, a double value that exceeds the range of `int` will saturate at Integer.MAX_VALUE. jshell> (int) ((17 * 4L + 2L) / 3L) $24 ==> -2028300629 jshell> (int) Math.ceil(17 / 0.75) $25 ==> 2147483647 Let's stick with the `Math.ceil` expression please. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On Fri, 11 Feb 2022 18:13:36 GMT, Roger Riggs wrote: >> @RogerRiggs >> so you recommend `(int) Math.min(((m.size() * 4L + 2L) / 3L), >> Integer.MAX_VALUE)`? OK then, changed it. >> please review again, thanks! > > Works for me. Thanks Just multiply by 0.75. On a modern design, floating-point multiply is 4 clocks latency, 4 ops/clock throughput. FP max is 2 clocks latency, conversions int-float and vice versa 3 clocks latency, 4 ops/clock throughput. Long division is 7-9 clocks, 2ops/clock throughput. Shift and add 2 clocks, 2/3 ops/clock througput. Compare is 1 clock, 3 ops/clock throughput, conditional move is 1 clock, 3 ops/clock throughput. Seems like it's a wash. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On Fri, 11 Feb 2022 17:04:14 GMT, XenoAmess wrote: >> Performance is a lesser issue. Given all of the other computation that >> occurs to populate the map, this computation is in the noise. It is also >> likely that with more instructions to fetch and execute and a possible >> branch, the integer check is slower. >> With current hardware, the long divide dominates the cost. Addition is >> almost free. >> >> Code maintainability is more important; keep the source code simple and >> concise. > > @RogerRiggs > so you recommend `(int) Math.min(((m.size() * 4L + 2L) / 3L), > Integer.MAX_VALUE)`? OK then, changed it. > please review again, thanks! Works for me. Thanks - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On Fri, 11 Feb 2022 16:32:29 GMT, Roger Riggs wrote:
>> @RogerRiggs
>>
>> Hi. I added a overflow check.
>>
>> The check makes sure it cannot overflow now.
>>
>> I wrote a test to show this overflow check be correct.
>>
>>
>> public class A {
>>
>> /**
>> * use for calculate HashMap capacity, using default load factor 0.75
>> *
>> * @param size size
>> * @return HashMap capacity under default load factor
>> */
>> private static int calculateHashMapCapacity(int size) {
>> if (size > (int) (Integer.MAX_VALUE * 0.75)) {
>> return Integer.MAX_VALUE;
>> }
>> return size + (size + 2) / 3;
>> }
>>
>> public static void main(String[] args) {
>> for (int i = 1; i < Integer.MAX_VALUE ; ++i) {
>> if (calculateHashMapCapacity(i) <= 0) {
>> System.err.println(i);
>> return;
>> }
>> }
>> }
>> }
>>
>>
>>
>> I also see the compiled result to make sure the `(int) (Integer.MAX_VALUE *
>> 0.75)` calculation is made at compiling but not runtime, which will not make
>> this function too much slower.
>>
>> please review again, thanks!
>
> Performance is a lesser issue. Given all of the other computation that occurs
> to populate the map, this computation is in the noise. It is also likely
> that with more instructions to fetch and execute and a possible branch, the
> integer check is slower.
> With current hardware, the long divide dominates the cost. Addition is
> almost free.
>
> Code maintainability is more important; keep the source code simple and
> concise.
@RogerRiggs
so you recommend `(int) Math.min(((m.size() * 4L + 2L) / 3L),
Integer.MAX_VALUE)`? OK then, changed it.
-
PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v5]
> 8281631: HashMap.putAll can cause redundant space waste XenoAmess has updated the pull request incrementally with one additional commit since the last revision: 9072610: HashMap.putAll can cause redundant space waste - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/2a1bf274..c6654478 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=03-04 Stats: 14 lines in 1 file changed: 0 ins; 13 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On Fri, 11 Feb 2022 15:48:44 GMT, XenoAmess wrote:
>> src/java.base/share/classes/java/util/WeakHashMap.java line 247:
>>
>>> 245: */
>>> 246: private static int calculateHashMapCapacity(int size){
>>> 247: return size + (size + 2) / 3;
>>
>> Consider integer overflow; if size were Integer.MAX_VALUE / 2; the
>> computation would overflow.
>> The negative result would eventually throw an exception. Using Long for the
>> computation would avoid that and keep the expression simple.
>
> @RogerRiggs
>
> Hi. I added a overflow check.
>
> The check makes sure it cannot overflow now.
>
> I wrote a test to show this overflow check be correct.
>
>
> public class A {
>
> /**
> * use for calculate HashMap capacity, using default load factor 0.75
> *
> * @param size size
> * @return HashMap capacity under default load factor
> */
> private static int calculateHashMapCapacity(int size) {
> if (size > (int) (Integer.MAX_VALUE * 0.75)) {
> return Integer.MAX_VALUE;
> }
> return size + (size + 2) / 3;
> }
>
> public static void main(String[] args) {
> for (int i = 1; i < Integer.MAX_VALUE ; ++i) {
> if (calculateHashMapCapacity(i) <= 0) {
> System.err.println(i);
> return;
> }
> }
> }
> }
>
>
>
> I also see the compiled result to make sure the `(int) (Integer.MAX_VALUE *
> 0.75)` calculation is made at compiling but not runtime, which will not make
> this function too much slower.
>
> please review again, thanks!
Performance is a lesser issue. Given all of the other computation that occurs
to populate the map, this computation is in the noise. It is also likely that
with more instructions to fetch and execute and a possible branch, the integer
check is slower.
With current hardware, the long divide dominates the cost. Addition is almost
free.
Code maintainability is more important; keep the source code simple and concise.
-
PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On Fri, 11 Feb 2022 15:04:03 GMT, Roger Riggs wrote:
>> XenoAmess has refreshed the contents of this pull request, and previous
>> commits have been removed. The incremental views will show differences
>> compared to the previous content of the PR. The pull request contains one
>> new commit since the last revision:
>>
>> 9072610: HashMap.putAll can cause redundant space waste
>
> src/java.base/share/classes/java/util/WeakHashMap.java line 247:
>
>> 245: */
>> 246: private static int calculateHashMapCapacity(int size){
>> 247: return size + (size + 2) / 3;
>
> Consider integer overflow; if size were Integer.MAX_VALUE / 2; the
> computation would overflow.
> The negative result would eventually throw an exception. Using Long for the
> computation would avoid that and keep the expression simple.
@RogerRiggs
Hi. I added a overflow check.
The check makes sure it cannot overflow now.
I wrote a test to show this overflow check be correct.
public class A {
/**
* use for calculate HashMap capacity, using default load factor 0.75
*
* @param size size
* @return HashMap capacity under default load factor
*/
private static int calculateHashMapCapacity(int size) {
if (size > (int) (Integer.MAX_VALUE * 0.75)) {
return Integer.MAX_VALUE;
}
return size + (size + 2) / 3;
}
public static void main(String[] args) {
for (int i = 1; i < Integer.MAX_VALUE ; ++i) {
if (calculateHashMapCapacity(i) <= 0) {
System.err.println(i);
return;
}
}
}
}
I also see the compiled result to make sure the `(int) (Integer.MAX_VALUE *
0.75)` calculation is made at compiling but not runtime, which will not make
this function too much slower.
please review again, thanks!
-
PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v4]
> 8281631: HashMap.putAll can cause redundant space waste XenoAmess has updated the pull request incrementally with one additional commit since the last revision: 9072610: HashMap.putAll can cause redundant space waste - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/bb42df9c..2a1bf274 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=02-03 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On Fri, 11 Feb 2022 15:04:03 GMT, Roger Riggs wrote: > if size were Integer.MAX_VALUE / 2; the computation would overflow Actually will not, it must be slightly larger. it will only overflow when it be larger than Integer.MAX_VALUE * 0.75 But yes, it can overflow when there be a map as large as that. > Using Long for the computation would avoid that and keep the expression > simple. but it be slower. I do think a int check can do same thing, and would add it . Thanks! - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
On Fri, 11 Feb 2022 13:04:38 GMT, XenoAmess wrote:
>> 8281631: HashMap.putAll can cause redundant space waste
>
> XenoAmess has refreshed the contents of this pull request, and previous
> commits have been removed. The incremental views will show differences
> compared to the previous content of the PR. The pull request contains one new
> commit since the last revision:
>
> 9072610: HashMap.putAll can cause redundant space waste
src/java.base/share/classes/java/util/WeakHashMap.java line 247:
> 245: */
> 246: private static int calculateHashMapCapacity(int size){
> 247: return size + (size + 2) / 3;
Consider integer overflow; if size were Integer.MAX_VALUE / 2; the computation
would overflow.
The negative result would eventually throw an exception. Using Long for the
computation would avoid that and keep the expression simple.
-
PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v3]
> 8281631: HashMap.putAll can cause redundant space waste XenoAmess has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 9072610: HashMap.putAll can cause redundant space waste - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/f18fc5e4..bb42df9c Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=01-02 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste [v2]
> 8281631: HashMap.putAll can cause redundant space waste XenoAmess has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 9072610: HashMap.putAll can cause redundant space waste - Changes: - all: https://git.openjdk.java.net/jdk/pull/7431/files - new: https://git.openjdk.java.net/jdk/pull/7431/files/b4098ac6..f18fc5e4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=00-01 Stats: 14 lines in 3 files changed: 10 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste
On Fri, 11 Feb 2022 11:55:13 GMT, stefan-zobel wrote: > > I investigated most of the usages. They just give a size, and get a > > capacity, even not change the 0.75 So maybe we can use some int calculation > > to replace the 0.75, thus replace Math.ceil for such situations. > > FWIW, `(int) Math.ceil(expected / 0.75)` and `(int) ((expected * 4L + 2L) / > 3L)` would be equivalent. Yes, and `(int) ((expected * 4L + 2L) / 3L)` actually equals `(expected + (expected + 2) / 3)` IMO, thus even avoid cast to long. - PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste
On Thu, 10 Feb 2022 18:09:19 GMT, XenoAmess wrote: > I investigated most of the usages. They just give a size, and get a capacity, > even not change the 0.75 So maybe we can use some int calculation to replace > the 0.75, thus replace Math.ceil for such situations. FWIW, `(int) Math.ceil(expected / 0.75)` and `(int) ((expected * 4L + 2L) / 3L)` would be equivalent. - PR: https://git.openjdk.java.net/jdk/pull/7431
RFR: 8281631: HashMap.putAll can cause redundant space waste
8281631: HashMap.putAll can cause redundant space waste - Commit messages: - 9072610: HashMap.putAll can cause redundant space waste Changes: https://git.openjdk.java.net/jdk/pull/7431/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7431&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281631 Stats: 8 lines in 2 files changed: 0 ins; 0 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/7431.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431 PR: https://git.openjdk.java.net/jdk/pull/7431
Re: RFR: 8281631: HashMap.putAll can cause redundant space waste
On Thu, 10 Feb 2022 17:46:36 GMT, XenoAmess wrote: > 8281631: HashMap.putAll can cause redundant space waste According to the discussion at mailing list, we decide to try only change the calculation inside HashMap and WeakHashMap, and see what would happen. The next step is fixing all such **size/0.75+1** in jdk (expected several hundreds places...) I investigated most of the usages. They just give a size, and get a capacity, even not change the 0.75 So maybe we can use some int calculation to replace the 0.75, thus replace Math.ceil for such situations. - PR: https://git.openjdk.java.net/jdk/pull/7431
