RFR: 8315454: Add a way to create an immutable snapshot of a BitSet [v4]

[Claes Redestad]

On Mon, 4 Sep 2023 06:54:40 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> This PR proposes adding a new method to BitSet that provides an immutable snapshot of the set in the form of an `IntPredicate`.
>> 
>> The predicate is eligible for constant folding.
>> 
>> Here are some classes in the JDK that would benefit directly from constant-folding of BitSets:
>> 
>> PoolReader (6)
>> URLEncoder (1) - Updated in this PR
>> HtmlTree (2)
>> 
>> More over, the implementation of the predicate is @ValueBased and this would provide additional benefits in the future.
>> 
>> Initial benchmarks with the URLEncoder show encouraging results:
>> 
>> 
>> Name                           (encodeChars) (maxLength) (unchanged) Cnt  Base   Error   Test   Error  Unit  Diff%
>> URLEncodeDecode.testEncodeUTF8             6        1024           0  15 2,371 ± 0,016  2,073 ± 0,184 ms/op  12,6% (p = 0,000*)
>> URLEncodeDecode.testEncodeUTF8             6        1024          75  15 1,772 ± 0,013  1,387 ± 0,032 ms/op  21,8% (p = 0,000*)
>> URLEncodeDecode.testEncodeUTF8             6        1024         100  15 1,230 ± 0,009  1,140 ± 0,011 ms/op   7,3% (p = 0,000*)
>
> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Do not propagate Integer.MAX_VALUE BitSets

Looks good to me. Since we're not adding any public API I think it's ok to include the `URLEncoder` changes so that we can use that benchmark as a proxy for verifying `BitSet::asPredicate` performance.

src/java.base/share/classes/jdk/internal/util/ImmutableBitSetPredicate.java line 60:

> 58:     public static IntPredicate of(BitSet original) {
> 59:         // Do not propagate the Integer.MAX_VALUE issue
> 60:         if (Integer.toUnsignedLong(original.length()) > Integer.MAX_VALUE) {

I'm not so sure about this. While there are issues related to `BitSet::set(Integer.MAX_VALUE)` (makes `BitSet::length` overflow to negative), the `get/test` operation is not adversely affected. Since this is an internal API adding limitations here seem a bit gratuitous and could instead be discussed if/when taking this public.

test/jdk/java/util/BitSet/ImmutableBitSet.java line 2:

> 1: /*
> 2:  * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.

Suggestion:

 * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.

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

Marked as reviewed by redestad (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/15530#pullrequestreview-1609364523
PR Review Comment: https://git.openjdk.org/jdk/pull/15530#discussion_r1314830577
PR Review Comment: https://git.openjdk.org/jdk/pull/15530#discussion_r1314830746