RFR: 8299444 java.util.Set.copyOf allocates needlessly for empty input collections

Viktor Klang duke at openjdk.org
Thu Jan 5 11:44:56 UTC 2023


On Thu, 5 Jan 2023 01:02:11 GMT, Sergey Bylokhov <serb at openjdk.org> wrote:

>> There's no regression test. However, with the current code (prior to this change) a call to `Set.of(zeroLengthArray)` returns the same instance as `Set.of()`, so it's difficult to write a simple functional test for this change. The test would have to assert that "no HashSet is allocated along this code path" which is much harder to test and frankly probably isn't worth it. So, please add one of the `noreg-*` labels to the bug to indicate the rationale for omitting a regression test.
>> 
>> https://openjdk.org/guide/#jbs-label-dictionary
>> 
>> I'd probably add the `Map.copyOf` change to this PR to avoid some bug/PR/review overhead. Thanks for mentioning this @shipilev.
>
>> so it's difficult to write a simple functional test for this change.
> 
> It is possible to track that for some "custom" and empty collection the only method will be called is `isEmpty` and nothing else. Not sure how it is useful or not.

@mrserb @stuart-marks Indeed, the reason for no regression test is because I didn't see any good way of testing "this operation will not allocate any memory" (which is the externally observable difference, besides having isEmpty being called, which of course could allocate memory (which I sincerely hope no implementations do)).

I can add the same logic to Map.copyOf, and possibly even List.copyOf (ImmutableCollections.listCopy). But will do so in additional commits in this PR.

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

PR: https://git.openjdk.org/jdk/pull/11847


More information about the core-libs-dev mailing list