On Wed, 18 Jan 2023 12:43:31 GMT, fabioromano1 <d...@openjdk.org> wrote:
>> The enanchment is useful for applications that make heavy use of BitSet >> objects as sets of integers, and therefore they need to make a lot of calls >> to cardinality() method, which actually require linear time in the number of >> words in use by the bit set. >> This optimization reduces the cost of calling cardinality() to constant >> time, as it simply returns the value of the field, and it also try to make >> as little effort as possible to update the field, when needed. >> >> Moreover, it has been implemented a new method for testing wheter a bit set >> includes another bit set (i.e., the set of true bits of the parameter is a >> subset of the true bits of the instance). > > fabioromano1 has updated the pull request incrementally with one additional > commit since the last revision: > > Added author and reverse the cicle order in includes(BitSet) Like other reviewers, changing the performance tradeoffs in BitSet make me uncomfortable. 30 years of code has adapted to the current performance tradeoffs. Those users who really need O(1) cardinality() can fairly easily implement it in client code (and probably have). The spirit of BitSet is space efficiency, and adding another field negates that. cardinality() is O(N) but in practice should be very efficient due to linear traversal and (presumably) the use of hardware bitcount instructions for each word (fix that if not true). There may be a case for a BitSet rewrite (e.g. to support large sparse bitsets) once we have compact value types. ------------- PR: https://git.openjdk.org/jdk/pull/11837
