from:"via GitHub"

Re: [PR] [COLLECTIONS-855] Fixed hashing calculation as per report [commons-collections]

2024-06-11 Thread via GitHub



aherbert commented on code in PR #501:
URL: 
https://github.com/apache/commons-collections/pull/501#discussion_r1634862521


##
src/main/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasher.java:
##
@@ -167,41 +167,33 @@ public boolean processIndices(final IntPredicate 
consumer) {
 // hash[i] = ( h1(x) - i*h2(x) - (i*i*i - i)/6 ) wrapped in 
[0, bits)
 
 int index = BitMaps.mod(initial, bits);
+if (!consumer.test(index)) {
+return false;
+}
 int inc = BitMaps.mod(increment, bits);
 
 final int k = shape.getNumberOfHashFunctions();
-if (k > bits) {
-for (int j = k; j > 0;) {
-// handle k > bits
-final int block = Math.min(j, bits);
-j -= block;
-for (int i = 1; i <= block; i++) {
-if (!consumer.test(index)) {
-return false;
-}
-// Update index and handle wrapping
-index -= inc;
-index = index < 0 ? index + bits : index;
-
-// Incorporate the counter into the increment to 
create a
-// tetrahedral number additional term, and handle 
wrapping.
-inc -= i;
-inc = inc < 0 ? inc + bits : inc;
-}
+
+// the tetraheadral incrementer.  We need to ensure that this
+// number does not exceed bits-1 or we may end up with an 
index > bits.
+int tet = 1;
+for (int i = 1; i < k; i++) {
+// Update index and handle wrapping
+index -= inc;
+index = index < 0 ? index + bits : index;
+if (!consumer.test(index)) {
+return false;
+}
+
+// Incorporate the counter into the increment to create a
+// tetrahedral number additional term, and handle wrapping.
+inc -= tet;
+inc = inc < 0 ? inc + bits : inc;
+if (inc >= bits) {
+inc = BitMaps.mod(increment, bits);

Review Comment:
   Don't do this. The entire purpose of the increment being tested against zero 
and adjusted by `[0, bits)` is to avoid a modulus inside the loop.
   
   Besides, you should be testing `tet` is within `[0, bits)`, not `inc`.
   
   I feel that the better solution is to have two versions. One for the idiot 
who requests more hash functions than there are bits in the filter; the other 
one optimised for the correct use case.
   



-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [COLLECTIONS-855] Fixed hashing calculation as per report [commons-collections]

2024-06-11 Thread via GitHub



Claudenw commented on PR #501:
URL: 
https://github.com/apache/commons-collections/pull/501#issuecomment-2160622978

   We do have coverage in the tests for the bad case.
   
   If we start an integer variable `int tet = 1;` and use it on line 198 rather 
than i.  We can then do 
   ```
   if (++tet == bits) {
   tet = 0;
   }
   ```
   to ensure that tet is always in the proper range.  Conceptually this is the 
same as calling modulus bits.  This means that we only have one loop to worry 
about.


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Bump org.apache.commons:commons-parent from 70 to 71 [commons-collections]

2024-06-11 Thread via GitHub



garydgregory merged PR #503:
URL: https://github.com/apache/commons-collections/pull/503


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Fix Reproducible Builds issues [commons-net]

2024-06-11 Thread via GitHub



garydgregory commented on PR #259:
URL: https://github.com/apache/commons-net/pull/259#issuecomment-2160544465

   


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [COLLECTIONS-855] Fixed hashing calculation as per report [commons-collections]

2024-06-11 Thread via GitHub



garydgregory commented on PR #501:
URL: 
https://github.com/apache/commons-collections/pull/501#issuecomment-2160537613

   Perhaps some of the information captured in this conversation could be 
preserved in a Java document or in-line comment.


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [COLLECTIONS-855] Fixed hashing calculation as per report [commons-collections]

2024-06-11 Thread via GitHub



aherbert commented on PR #501:
URL: 
https://github.com/apache/commons-collections/pull/501#issuecomment-2160492067

   The increment `inc` is adjusted by `i`. If it is negative then it has 
wrapped and we add `bits`. This works only if `i` is always less than `bits`.
   
   If `i > bits` then when we adjust `inc` by adding `bits` it may still be 
negative. This is then an invalid increment to use to adjust `index`.
   
   In the worst case the `inc` will be a large enough negative that `index -= 
inc` will great a number above the size in `bits`. That could cause a bounds 
error on the IntPredicate consumer.
   
   In the real world the upper loop will not be used. It is a fail safe for bad 
use. A number of hash functions greater than the number of bits would saturate 
the filter very fast. An alternative less friendly solution would be to throw 
an IllegalArgumentException if called under these conditions.
   
   As to resetting the tetrahedral number then you are correct. My fail-safe 
implementation is wrong for a correct enhanced double hasher. What the upper 
loop requires is for the `inc + bits` to be repeated until `inc` is positive:
   ```java
   for (int i = 1; i <= k; i++) {
   if (!consumer.test(index)) {
   return false;
   }
   // Update index and handle wrapping
   index -= inc;
   index = index < 0 ? index + bits : index;
   
   // Incorporate the counter into the increment to 
create a
   // tetrahedral number additional term, and handle 
wrapping
   // **given** i can exceed bits
   inc -= i;
   if (inc < 0) {
   inc += bits;
   while (inc < 0) {
   inc += bits;
   }
   }
   }
   ```
   
   Try that and see if we have coverage. If not then we should add a test to 
make sure that we are testing extremely bad usage and the hasher still works.
   


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [COLLECTIONS-855] Fixed hashing calculation as per report [commons-collections]

2024-06-11 Thread via GitHub



Claudenw commented on PR #501:
URL: 
https://github.com/apache/commons-collections/pull/501#issuecomment-2160483512

   OK, I figured out what the lines 173-192 are doing .  They ensure that the 
`inc` does not get greater than `bits`. 


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [COLLECTIONS-855] Fixed hashing calculation as per report [commons-collections]

2024-06-11 Thread via GitHub



Claudenw commented on PR #501:
URL: 
https://github.com/apache/commons-collections/pull/501#issuecomment-2160361892

   I think this block is wrong: 
https://github.com/apache/commons-collections/blob/bb2fac47dc24033bae8a180bc59cb1e6e8769318/src/main/java/org/apache/commons/collections4/bloomfilter/EnhancedDoubleHasher.java#L172-L207
   
   In the upper part we reset the tetrahedral number counter so the hashing 
algo is reset.
   I don't recall why the algo is split into k<=bits and k>bits, the wrapping 
algorithm should work for either case (I think).  What am I missing?  Can this 
be simplified by dropping lines 172-192 inclusive?
   
   I will make the adjustment that @aherbert suggested after we resolve this.


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] LANG-1728: Deprecate EvenListenerSupport [commons-lang]

2024-06-11 Thread via GitHub



thachlp commented on code in PR #1229:
URL: https://github.com/apache/commons-lang/pull/1229#discussion_r1634357210


##
src/main/java/org/apache/commons/lang3/event/EventListenerSupport.java:
##
@@ -66,9 +66,14 @@
  * 
  *
  * @param  the type of event listener that is supported by this proxy.
+ * @deprecated This class is deprecated because it does not specifically 
relate to event listeners. It primarily allows
+ * sequential invocation of the same method on each member of a list of 
objects of the same type. Modern Java features such
+ * as lambdas and executors provide more flexible and efficient ways to 
achieve this functionality.
+ * Consider using lambdas or executors for similar functionality.
  *
  * @since 3.0
  */
+@Deprecated
 public class EventListenerSupport implements Serializable {

Review Comment:
   Hi @garydgregory, could you please help review, if it is not necessary, I 
will close it, thanks 



-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Fix Reproducible Builds issues [commons-net]

2024-06-10 Thread via GitHub



hboutemy commented on PR #259:
URL: https://github.com/apache/commons-net/pull/259#issuecomment-2159810198

   great, first fully reproducible release done: 
https://github.com/jvm-repo-rebuild/reproducible-central/blob/master/content/org/apache/commons/net/commons-net-3.11.1.buildcompare


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Bump commons-net:commons-net from 3.11.0 to 3.11.1 [commons-vfs]

2024-06-10 Thread via GitHub



garydgregory merged PR #548:
URL: https://github.com/apache/commons-vfs/pull/548


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[PR] Improve performance of UriParser normalize [commons-vfs]

2024-06-10 Thread via GitHub

japplis opened a new pull request, #547:
URL: https://github.com/apache/commons-vfs/pull/547

Here is a performance improvement of `UriParser.normalise()`.
I've replaced the slow `StringBuilder.substring()` with
`StringBuilder.charAt()`. I think that `StringBuilder.subSequence()` with
`CharSequence.compare()` could show similar performance improvements. Let me
know if you want me to test this way.

Note that I would advise to first merge
https://github.com/apache/commons-vfs/pull/543/files that fixes a bug in the
test of %2f. I didn't want to reintroduce the bug with this merge request so
the bug is also fixed in here. This MR may have a merge conflict with
https://github.com/apache/commons-vfs/pull/543/ as it touches the same part of
the code. If you want I can resolve it once
https://github.com/apache/commons-vfs/pull/543/ is merged.

On my machine (Desktop from 2016) I have a 3x throughput.

JMH
Benchmark Mode Cnt Score Error Units
UriParserBenchmark.normalise thrpt 50 994208,463 11590,176 ops/s

After refactoring
Benchmark Mode CntScore Error Units
UriParserBenchmark.normalise thrpt 25 2940553,554 47617,743 ops/s

JFR/JMC flamegraphs

![profiling-UriParser-crop](https://github.com/apache/commons-vfs/assets/318821/aca64859-e509-4280-859f-92a8e6b32fde)

![profiling-UriParser2-crop](https://github.com/apache/commons-vfs/assets/318821/79cd4ccc-c7a2-4aca-8df9-50a067dfcd59)

--
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Add multiple utility methods to StringUtils in Apache Commons Lang [commons-lang]

2024-06-10 Thread via GitHub



GERONlMO closed pull request #1231: Add multiple utility methods to StringUtils 
in Apache Commons Lang
URL: https://github.com/apache/commons-lang/pull/1231


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] [COLLECTIONS-855] Fixed hashing calculation as per report [commons-collections]

2024-06-09 Thread via GitHub



garydgregory commented on PR #501:
URL: 
https://github.com/apache/commons-collections/pull/501#issuecomment-2156636809

   https://issues.apache.org/jira/browse/COLLECTIONS-855


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] COLLECTIONS-855: Fixed hashing calculation as per report [commons-collections]

2024-06-09 Thread via GitHub



garydgregory commented on PR #501:
URL: 
https://github.com/apache/commons-collections/pull/501#issuecomment-2156636601

   @Claudenw ?


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Bump org.easymock:easymock from 5.2.0 to 5.3.0 [commons-lang]

2024-06-09 Thread via GitHub



garydgregory merged PR #1232:
URL: https://github.com/apache/commons-lang/pull/1232


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] Bump org.easymock:easymock from 5.2.0 to 5.3.0 [commons-collections]

2024-06-09 Thread via GitHub



garydgregory merged PR #502:
URL: https://github.com/apache/commons-collections/pull/502


-- 
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-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

1 2 3 4 5 6 7 8 9 10 >

1 - 100 of 8301 matches

Mail list logo