You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/08/09 07:45:40 UTC
[GitHub] [commons-collections] Claudenw opened a new pull request, #329: Collections-818: convert to characteristics flag
Claudenw opened a new pull request, #329:
URL: https://github.com/apache/commons-collections/pull/329
converted BloomFilter.isSparse() to BloomFilter.characteristics() and defiend SPARSE constant.
fixes Collections-818
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] Claudenw commented on a diff in pull request #329: Collections-818: convert to characteristics flag
Posted by GitBox <gi...@apache.org>.
Claudenw commented on code in PR #329:
URL: https://github.com/apache/commons-collections/pull/329#discussion_r946467502
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -69,7 +73,7 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
*/
default boolean contains(BloomFilter other) {
Objects.requireNonNull(other, "other");
- return isSparse() ? contains((IndexProducer) other) : contains((BitMapProducer) other);
+ return (characteristics()&SPARSE)>0 ? contains((IndexProducer) other) : contains((BitMapProducer) other);
Review Comment:
fixed
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilter.java:
##########
@@ -65,7 +65,7 @@ public SimpleBloomFilter(BloomFilter other) {
this.shape = other.getShape();
this.bitMap = new long[BitMap.numberOfBitMaps(shape.getNumberOfBits())];
this.cardinality = 0;
- if (other.isSparse()) {
+ if ((other.characteristics()&SPARSE)>0) {
Review Comment:
fixed
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] aherbert commented on a diff in pull request #329: Collections-818: convert to characteristics flag
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #329:
URL: https://github.com/apache/commons-collections/pull/329#discussion_r941544205
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -29,6 +29,15 @@
*/
public interface BloomFilter extends IndexProducer, BitMapProducer {
+ /**
+ * The sparse characteristic used to determine the best method for matching.
+ * <p>For `sparse` implementations
+ * the {@code forEachIndex(IntConsumer consumer)} method is more efficient. For non `sparse` implementations
+ * the {@code forEachBitMap(LongConsumer consumer)} is more efficient. Implementers should determine if it is easier
+ * for the implementation to produce indexes of bit map blocks.</p>
+ */
+ int SPARSE=0x1;
Review Comment:
`SPARSE = 0x1`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -38,17 +47,12 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
// Query Operations
/**
- * This method is used to determine the best method for matching.
- *
- * <p>For `sparse` implementations
- * the {@code forEachIndex(IntConsumer consumer)} method is more efficient. For non `sparse` implementations
- * the {@code forEachBitMap(LongConsumer consumer)} is more efficient. Implementers should determine if it is easier
- * for the implementation to produce indexes of bit map blocks.</p>
- *
- * @return {@code true} if the implementation is sparse {@code false} otherwise.
- * @see BitMap
+ * Returns the bitmap of characteristics of the filter.
Review Comment:
Drop the `bitmap of`. I think it confusing as `bitmap` in this package is specifically used to refer to a range of indices [0, 63] packed into a long.
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -38,17 +47,12 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
// Query Operations
/**
- * This method is used to determine the best method for matching.
- *
- * <p>For `sparse` implementations
- * the {@code forEachIndex(IntConsumer consumer)} method is more efficient. For non `sparse` implementations
- * the {@code forEachBitMap(LongConsumer consumer)} is more efficient. Implementers should determine if it is easier
- * for the implementation to produce indexes of bit map blocks.</p>
- *
- * @return {@code true} if the implementation is sparse {@code false} otherwise.
- * @see BitMap
+ * Returns the bitmap of characteristics of the filter.
+ * <p>
+ * Characteristics are defined as bits witin the characteristics integer.
Review Comment:
`within`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilter.java:
##########
@@ -194,7 +194,7 @@ public boolean merge(Hasher hasher) {
@Override
public boolean merge(BloomFilter other) {
Objects.requireNonNull(other, "other");
- if (other.isSparse()) {
+ if ((other.characteristics()&SPARSE)>0) {
Review Comment:
whitespace: `(other.characteristics() & SPARSE) != 0`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SparseBloomFilter.java:
##########
@@ -58,7 +58,7 @@ public SparseBloomFilter(BloomFilter other) {
Objects.requireNonNull(other, "other");
this.shape = other.getShape();
this.indices = new TreeSet<>();
- if (other.isSparse()) {
+ if ((other.characteristics()&SPARSE)>0) {
Review Comment:
whitespace: `(other.characteristics() & SPARSE) != 0`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SparseBloomFilter.java:
##########
@@ -169,7 +169,7 @@ public boolean merge(Hasher hasher) {
@Override
public boolean merge(BloomFilter other) {
Objects.requireNonNull(other, "other");
- IndexProducer producer = other.isSparse() ? (IndexProducer) other : IndexProducer.fromBitMapProducer(other);
+ IndexProducer producer = (other.characteristics()&SPARSE)>0 ? (IndexProducer) other : IndexProducer.fromBitMapProducer(other);
Review Comment:
whitespace: `(other.characteristics() & SPARSE) != 0`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilter.java:
##########
@@ -65,7 +65,7 @@ public SimpleBloomFilter(BloomFilter other) {
this.shape = other.getShape();
this.bitMap = new long[BitMap.numberOfBitMaps(shape.getNumberOfBits())];
this.cardinality = 0;
- if (other.isSparse()) {
+ if ((other.characteristics()&SPARSE)>0) {
Review Comment:
whitespace: `(other.characteristics() & SPARSE) != 0`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -69,7 +73,7 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
*/
default boolean contains(BloomFilter other) {
Objects.requireNonNull(other, "other");
- return isSparse() ? contains((IndexProducer) other) : contains((BitMapProducer) other);
+ return (characteristics()&SPARSE)>0 ? contains((IndexProducer) other) : contains((BitMapProducer) other);
Review Comment:
whitespace: `(characteristics() & SPARSE) != 0`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -144,7 +148,7 @@ default boolean merge(Hasher hasher) {
Objects.requireNonNull(hasher, "hasher");
Shape shape = getShape();
// create the bloomfilter that is most likely to merge quickly with this one
- BloomFilter result = isSparse() ? new SparseBloomFilter(shape, hasher) : new SimpleBloomFilter(shape, hasher);
+ BloomFilter result = (characteristics()&SPARSE)>0 ? new SparseBloomFilter(shape, hasher) : new SimpleBloomFilter(shape, hasher);
Review Comment:
whitespace: `(characteristics() & SPARSE) != 0`
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] Claudenw commented on a diff in pull request #329: Collections-818: convert to characteristics flag
Posted by GitBox <gi...@apache.org>.
Claudenw commented on code in PR #329:
URL: https://github.com/apache/commons-collections/pull/329#discussion_r946467749
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SparseBloomFilter.java:
##########
@@ -169,7 +169,7 @@ public boolean merge(Hasher hasher) {
@Override
public boolean merge(BloomFilter other) {
Objects.requireNonNull(other, "other");
- IndexProducer producer = other.isSparse() ? (IndexProducer) other : IndexProducer.fromBitMapProducer(other);
+ IndexProducer producer = (other.characteristics()&SPARSE)>0 ? (IndexProducer) other : IndexProducer.fromBitMapProducer(other);
Review Comment:
fixed
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] Claudenw commented on a diff in pull request #329: Collections-818: convert to characteristics flag
Posted by GitBox <gi...@apache.org>.
Claudenw commented on code in PR #329:
URL: https://github.com/apache/commons-collections/pull/329#discussion_r946467184
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -29,6 +29,15 @@
*/
public interface BloomFilter extends IndexProducer, BitMapProducer {
+ /**
+ * The sparse characteristic used to determine the best method for matching.
+ * <p>For `sparse` implementations
+ * the {@code forEachIndex(IntConsumer consumer)} method is more efficient. For non `sparse` implementations
+ * the {@code forEachBitMap(LongConsumer consumer)} is more efficient. Implementers should determine if it is easier
+ * for the implementation to produce indexes of bit map blocks.</p>
+ */
+ int SPARSE=0x1;
Review Comment:
fixed
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -38,17 +47,12 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
// Query Operations
/**
- * This method is used to determine the best method for matching.
- *
- * <p>For `sparse` implementations
- * the {@code forEachIndex(IntConsumer consumer)} method is more efficient. For non `sparse` implementations
- * the {@code forEachBitMap(LongConsumer consumer)} is more efficient. Implementers should determine if it is easier
- * for the implementation to produce indexes of bit map blocks.</p>
- *
- * @return {@code true} if the implementation is sparse {@code false} otherwise.
- * @see BitMap
+ * Returns the bitmap of characteristics of the filter.
+ * <p>
+ * Characteristics are defined as bits witin the characteristics integer.
Review Comment:
fixed
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] aherbert merged pull request #329: Collections-818: convert to characteristics flag
Posted by GitBox <gi...@apache.org>.
aherbert merged PR #329:
URL: https://github.com/apache/commons-collections/pull/329
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] codecov-commenter commented on pull request #329: Collections-818: convert to characteristics flag
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #329:
URL: https://github.com/apache/commons-collections/pull/329#issuecomment-1209039539
# [Codecov](https://codecov.io/gh/apache/commons-collections/pull/329?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#329](https://codecov.io/gh/apache/commons-collections/pull/329?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (853ae39) into [master](https://codecov.io/gh/apache/commons-collections/commit/df091173cdfabd5ecc852f47c978ee9bcb2b7059?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (df09117) will **decrease** coverage by `0.05%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #329 +/- ##
============================================
- Coverage 86.01% 85.96% -0.06%
+ Complexity 4675 4673 -2
============================================
Files 288 288
Lines 13473 13473
Branches 1980 1980
============================================
- Hits 11589 11582 -7
- Misses 1323 1327 +4
- Partials 561 564 +3
```
| [Impacted Files](https://codecov.io/gh/apache/commons-collections/pull/329?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ections4/bloomfilter/ArrayCountingBloomFilter.java](https://codecov.io/gh/apache/commons-collections/pull/329/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29sbGVjdGlvbnM0L2Jsb29tZmlsdGVyL0FycmF5Q291bnRpbmdCbG9vbUZpbHRlci5qYXZh) | `100.00% <100.00%> (ø)` | |
| [.../commons/collections4/bloomfilter/BloomFilter.java](https://codecov.io/gh/apache/commons-collections/pull/329/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29sbGVjdGlvbnM0L2Jsb29tZmlsdGVyL0Jsb29tRmlsdGVyLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...ns/collections4/bloomfilter/SimpleBloomFilter.java](https://codecov.io/gh/apache/commons-collections/pull/329/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29sbGVjdGlvbnM0L2Jsb29tZmlsdGVyL1NpbXBsZUJsb29tRmlsdGVyLmphdmE=) | `94.25% <100.00%> (ø)` | |
| [...ns/collections4/bloomfilter/SparseBloomFilter.java](https://codecov.io/gh/apache/commons-collections/pull/329/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29sbGVjdGlvbnM0L2Jsb29tZmlsdGVyL1NwYXJzZUJsb29tRmlsdGVyLmphdmE=) | `96.66% <100.00%> (ø)` | |
| [...commons/collections4/map/AbstractReferenceMap.java](https://codecov.io/gh/apache/commons-collections/pull/329/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29sbGVjdGlvbnM0L21hcC9BYnN0cmFjdFJlZmVyZW5jZU1hcC5qYXZh) | `88.88% <0.00%> (-2.60%)` | :arrow_down: |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. [Learn more](https://about.codecov.io/product/feature/runtime-insights/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] Claudenw commented on a diff in pull request #329: Collections-818: convert to characteristics flag
Posted by GitBox <gi...@apache.org>.
Claudenw commented on code in PR #329:
URL: https://github.com/apache/commons-collections/pull/329#discussion_r946468053
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -38,17 +47,12 @@ public interface BloomFilter extends IndexProducer, BitMapProducer {
// Query Operations
/**
- * This method is used to determine the best method for matching.
- *
- * <p>For `sparse` implementations
- * the {@code forEachIndex(IntConsumer consumer)} method is more efficient. For non `sparse` implementations
- * the {@code forEachBitMap(LongConsumer consumer)} is more efficient. Implementers should determine if it is easier
- * for the implementation to produce indexes of bit map blocks.</p>
- *
- * @return {@code true} if the implementation is sparse {@code false} otherwise.
- * @see BitMap
+ * Returns the bitmap of characteristics of the filter.
Review Comment:
fixed
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -144,7 +148,7 @@ default boolean merge(Hasher hasher) {
Objects.requireNonNull(hasher, "hasher");
Shape shape = getShape();
// create the bloomfilter that is most likely to merge quickly with this one
- BloomFilter result = isSparse() ? new SparseBloomFilter(shape, hasher) : new SimpleBloomFilter(shape, hasher);
+ BloomFilter result = (characteristics()&SPARSE)>0 ? new SparseBloomFilter(shape, hasher) : new SimpleBloomFilter(shape, hasher);
Review Comment:
fixed
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] Claudenw commented on a diff in pull request #329: Collections-818: convert to characteristics flag
Posted by GitBox <gi...@apache.org>.
Claudenw commented on code in PR #329:
URL: https://github.com/apache/commons-collections/pull/329#discussion_r946468361
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilter.java:
##########
@@ -194,7 +194,7 @@ public boolean merge(Hasher hasher) {
@Override
public boolean merge(BloomFilter other) {
Objects.requireNonNull(other, "other");
- if (other.isSparse()) {
+ if ((other.characteristics()&SPARSE)>0) {
Review Comment:
fixed
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SparseBloomFilter.java:
##########
@@ -58,7 +58,7 @@ public SparseBloomFilter(BloomFilter other) {
Objects.requireNonNull(other, "other");
this.shape = other.getShape();
this.indices = new TreeSet<>();
- if (other.isSparse()) {
+ if ((other.characteristics()&SPARSE)>0) {
Review Comment:
fixed
--
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: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org