lhotari commented on code in PR #26128: URL: https://github.com/apache/pulsar/pull/26128#discussion_r3519775515
########## pulsar-common/src/main/java/org/apache/pulsar/common/util/AckSetUtil.java: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.common.util; + +/** + * Utility methods for operating on ack-set word arrays without allocating a {@code BitSet} instance. + */ Review Comment: This could be generalized so that it's about operating on "a long array containing a little-endian representation of all the bits in a bit set". There could be javadoc links to `java.util.BitSet#toLongArray` and `java.util.BitSet#valueOf(long[])` as a reference. ########## pulsar-common/src/main/java/org/apache/pulsar/common/util/AckSetUtil.java: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.common.util; + +/** + * Utility methods for operating on ack-set word arrays without allocating a {@code BitSet} instance. + */ +public class AckSetUtil { Review Comment: This class could be called `LongArrayBitSets`. In some code locations, the concept for a BitSet in `long[]` word representation is called "long array". I guess this concept comes from `BitSet.toLongArray` method. I prefer a plural format for the class name of a class that contains static methods for handling specific concepts. That's why I'd avoid the `Util` suffix. It also directs later maintainers so that this class doesn't become a container for all sorts of utility methods. ########## pulsar-common/src/main/java/org/apache/pulsar/common/util/AckSetUtil.java: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.common.util; + +/** + * Utility methods for operating on ack-set word arrays without allocating a {@code BitSet} instance. + */ +public class AckSetUtil { + Review Comment: Add a private constructor so that this class cannot be instantiated ########## pulsar-common/src/main/java/org/apache/pulsar/common/util/AckSetUtil.java: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.common.util; + +/** + * Utility methods for operating on ack-set word arrays without allocating a {@code BitSet} instance. + */ +public class AckSetUtil { + + /** + * Returns the number of bits set to {@code true} in the given words. + * + * @param words a long array containing a little-endian representation of a sequence of bits + * @return the number of bits set to {@code true} + */ + public static int cardinality(long[] words) { + int sum = 0; + for (long word : words) { + sum += Long.bitCount(word); + } + return sum; + } + + /** + * Returns a new word array whose elements are the bitwise AND of the corresponding elements of the two inputs. + * + * <p>Extra words in the longer array are implicitly ANDed with zero and therefore contribute no set bits. + * Trailing all-zero words are trimmed from the result, so it is in the same canonical form produced by + * {@link java.util.BitSet#toLongArray()}. + * + * @param set1 a long array containing a little-endian representation of a sequence of bits + * @param set2 a long array containing a little-endian representation of a sequence of bits + * @return a new array representing the intersection of the two bit sets, with trailing zero words trimmed + */ + public static long[] intersect(long[] set1, long[] set2) { + int len = Math.min(set1.length, set2.length); + while (len > 0 && (set1[len - 1] & set2[len - 1]) == 0) { + len--; + } + long[] result = new long[len]; + for (int i = 0; i < len; i++) { + result[i] = set1[i] & set2[i]; + } + return result; + } + + /** + * Returns the number of bits set to {@code true} after applying a logical <b>AND</b> to the given words. + * + * <p>When the arrays differ in length, extra words in the longer array are treated as zero (i.e. the AND + * result for those positions is zero and contributes nothing to the cardinality). + * + * @param set1 a long array containing a little-endian representation of a sequence of bits + * @param set2 a long array containing a little-endian representation of a sequence of bits + * @return the number of bits set to {@code true} in {@code set1 & set2} + */ + public static int cardinalityOfIntersection(long[] set1, long[] set2) { Review Comment: rename parameters ########## pulsar-common/src/main/java/org/apache/pulsar/common/util/AckSetUtil.java: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.common.util; + +/** + * Utility methods for operating on ack-set word arrays without allocating a {@code BitSet} instance. + */ +public class AckSetUtil { + + /** + * Returns the number of bits set to {@code true} in the given words. + * + * @param words a long array containing a little-endian representation of a sequence of bits + * @return the number of bits set to {@code true} + */ + public static int cardinality(long[] words) { Review Comment: words -> bitSet, to be consistent with the "long array bit set" concept ########## pulsar-common/src/main/java/org/apache/pulsar/common/util/AckSetUtil.java: ########## @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pulsar.common.util; + +/** + * Utility methods for operating on ack-set word arrays without allocating a {@code BitSet} instance. + */ +public class AckSetUtil { + + /** + * Returns the number of bits set to {@code true} in the given words. + * + * @param words a long array containing a little-endian representation of a sequence of bits + * @return the number of bits set to {@code true} + */ + public static int cardinality(long[] words) { + int sum = 0; + for (long word : words) { + sum += Long.bitCount(word); + } + return sum; + } + + /** + * Returns a new word array whose elements are the bitwise AND of the corresponding elements of the two inputs. + * + * <p>Extra words in the longer array are implicitly ANDed with zero and therefore contribute no set bits. + * Trailing all-zero words are trimmed from the result, so it is in the same canonical form produced by + * {@link java.util.BitSet#toLongArray()}. + * + * @param set1 a long array containing a little-endian representation of a sequence of bits + * @param set2 a long array containing a little-endian representation of a sequence of bits + * @return a new array representing the intersection of the two bit sets, with trailing zero words trimmed + */ + public static long[] intersect(long[] set1, long[] set2) { Review Comment: perhaps it would be more accurate to rename the parameters to `bitSet1` and `bitSet2`. the `long[]` type already makes it clear that these are "long array bit sets" -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
