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/06/29 08:38:06 UTC
[GitHub] [commons-collections] Claudenw opened a new pull request, #316: Modified forEachBitMap
Claudenw opened a new pull request, #316:
URL: https://github.com/apache/commons-collections/pull/316
Resolves https://issues.apache.org/jira/projects/COLLECTIONS/issues/COLLECTIONS-823
Modifies `ArrayCountingBloomFilter.forEachBitmap()` to be more efficient. by not creating an array of longs but building each long and calling the consumer in turn.
--
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 #316: Modified forEachBitMap
Posted by GitBox <gi...@apache.org>.
Claudenw commented on code in PR #316:
URL: https://github.com/apache/commons-collections/pull/316#discussion_r912462814
##########
src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java:
##########
@@ -231,7 +231,29 @@ public boolean forEachIndex(IntPredicate consumer) {
@Override
public boolean forEachBitMap(LongPredicate consumer) {
Objects.requireNonNull(consumer, "consumer");
- return BitMapProducer.fromIndexProducer(this, shape.getNumberOfBits()).forEachBitMap(consumer);
+ int blocksm1 = BitMap.numberOfBitMaps(shape.getNumberOfBits()) - 1;
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 #316: Modified forEachBitMap
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #316:
URL: https://github.com/apache/commons-collections/pull/316#discussion_r909376788
##########
src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java:
##########
@@ -231,7 +231,29 @@ public boolean forEachIndex(IntPredicate consumer) {
@Override
public boolean forEachBitMap(LongPredicate consumer) {
Objects.requireNonNull(consumer, "consumer");
- return BitMapProducer.fromIndexProducer(this, shape.getNumberOfBits()).forEachBitMap(consumer);
+ int blocksm1 = BitMap.numberOfBitMaps(shape.getNumberOfBits()) - 1;
Review Comment:
`final int blocksm1`
Although it is the same number I would prefer `counts.length` here and not `shape.getNumberOfBits()` since it is the total length of the counts array that is being iterated over.
--
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 #316: Modified forEachBitMap
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #316:
URL: https://github.com/apache/commons-collections/pull/316#discussion_r909380561
##########
src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java:
##########
@@ -231,7 +231,29 @@ public boolean forEachIndex(IntPredicate consumer) {
@Override
public boolean forEachBitMap(LongPredicate consumer) {
Objects.requireNonNull(consumer, "consumer");
- return BitMapProducer.fromIndexProducer(this, shape.getNumberOfBits()).forEachBitMap(consumer);
+ int blocksm1 = BitMap.numberOfBitMaps(shape.getNumberOfBits()) - 1;
Review Comment:
`final int blocksm1`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java:
##########
@@ -231,7 +231,29 @@ public boolean forEachIndex(IntPredicate consumer) {
@Override
public boolean forEachBitMap(LongPredicate consumer) {
Objects.requireNonNull(consumer, "consumer");
- return BitMapProducer.fromIndexProducer(this, shape.getNumberOfBits()).forEachBitMap(consumer);
+ int blocksm1 = BitMap.numberOfBitMaps(shape.getNumberOfBits()) - 1;
+ int i = 0;
+ long value;
+ // must break final block separate as the the number of bits may not fall on the long boundary
Review Comment:
Remove `the the`
##########
src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java:
##########
@@ -231,7 +231,29 @@ public boolean forEachIndex(IntPredicate consumer) {
@Override
public boolean forEachBitMap(LongPredicate consumer) {
Objects.requireNonNull(consumer, "consumer");
- return BitMapProducer.fromIndexProducer(this, shape.getNumberOfBits()).forEachBitMap(consumer);
+ int blocksm1 = BitMap.numberOfBitMaps(shape.getNumberOfBits()) - 1;
Review Comment:
Although it is the same number I would prefer `counts.length` here and not `shape.getNumberOfBits()` since it is the total length of the counts array that is being iterated over.
--
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 #316: COLLECTIONS-823: Modified ArrayCountingBloomFilter.forEachBitMap to be more efficient
Posted by GitBox <gi...@apache.org>.
aherbert merged PR #316:
URL: https://github.com/apache/commons-collections/pull/316
--
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 #316: Modified forEachBitMap
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #316:
URL: https://github.com/apache/commons-collections/pull/316#issuecomment-1169738576
# [Codecov](https://codecov.io/gh/apache/commons-collections/pull/316?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 [#316](https://codecov.io/gh/apache/commons-collections/pull/316?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (aa49cdc) into [master](https://codecov.io/gh/apache/commons-collections/commit/a79ea005b1ec4ad228067a9c5f2e10dcfb324c5e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a79ea00) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #316 +/- ##
============================================
+ Coverage 86.07% 86.09% +0.01%
- Complexity 4673 4679 +6
============================================
Files 287 287
Lines 13511 13524 +13
Branches 1983 1989 +6
============================================
+ Hits 11630 11643 +13
Misses 1321 1321
Partials 560 560
```
| [Impacted Files](https://codecov.io/gh/apache/commons-collections/pull/316?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/316/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%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/commons-collections/pull/316?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/commons-collections/pull/316?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a79ea00...aa49cdc](https://codecov.io/gh/apache/commons-collections/pull/316?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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 #316: Modified forEachBitMap
Posted by GitBox <gi...@apache.org>.
Claudenw commented on code in PR #316:
URL: https://github.com/apache/commons-collections/pull/316#discussion_r909395880
##########
src/main/java/org/apache/commons/collections4/bloomfilter/ArrayCountingBloomFilter.java:
##########
@@ -231,7 +231,29 @@ public boolean forEachIndex(IntPredicate consumer) {
@Override
public boolean forEachBitMap(LongPredicate consumer) {
Objects.requireNonNull(consumer, "consumer");
- return BitMapProducer.fromIndexProducer(this, shape.getNumberOfBits()).forEachBitMap(consumer);
+ int blocksm1 = BitMap.numberOfBitMaps(shape.getNumberOfBits()) - 1;
+ int i = 0;
+ long value;
+ // must break final block separate as the the number of bits may not fall on the long boundary
Review Comment:
done
--
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