date:20231104

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

2023-11-04 Thread via GitHub



dungba88 commented on PR #12738:
URL: https://github.com/apache/lucene/pull/12738#issuecomment-1793580112

   Thank you @mikemccand ! Agree we should have a single changes entry 
summarizing all different PR


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



rmuir merged PR #12737:
URL: https://github.com/apache/lucene/pull/12737


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Streamline FST constructors and make it fully read-only [lucene]

2023-11-04 Thread via GitHub



dungba88 commented on code in PR #12758:
URL: https://github.com/apache/lucene/pull/12758#discussion_r1382474566


##
lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java:
##
@@ -828,6 +829,26 @@ public void add(IntsRef input, T output) throws 
IOException {
 lastInput.copyInts(input);
   }
 
+  void setEmptyOutput(T v) {
+if (fst.metadata.emptyOutput != null) {
+  fst.metadata.emptyOutput = fst.outputs.merge(fst.metadata.emptyOutput, 
v);

Review Comment:
   startNode & numBytes cannot be final as well, as they can only be known at 
the end.



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Make FST fully read-only and streamline FST constructor [lucene]

2023-11-04 Thread via GitHub



dungba88 commented on PR #12691:
URL: https://github.com/apache/lucene/pull/12691#issuecomment-1793570275

   Yeah, this PR was originally opened for another purpose: consolidate the 
FSTStore and BytesStore, and that was already 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-unsubscr...@lucene.apache.org

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



uschindler commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793568091

   Sorry, pressed wrong button. Reopened.


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



uschindler closed pull request #12737: Speed up vectorutil float scalar 
methods, unroll properly, use fma where possible
URL: https://github.com/apache/lucene/pull/12737


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Streamline FST constructors and make it fully read-only [lucene]

2023-11-04 Thread via GitHub



dungba88 commented on code in PR #12758:
URL: https://github.com/apache/lucene/pull/12758#discussion_r1382472928


##
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##
@@ -1132,4 +1137,28 @@ public abstract static class BytesReader extends 
DataInput {
 /** Returns true if this reader uses reversed bytes under-the-hood. */
 public abstract boolean reversed();
   }
+
+  /**
+   * Represent the FST metadata
+   *
+   * @param  the FST output type
+   */
+  public static class FSTMetadata {

Review Comment:
   > This is public because callers would need to separately write/read the 
FSTMetadata some their sources? The FST bytes can stream append only to disk, 
but the end, the compiler produces the final FSTMetadata which must then be 
stored somewhere?
   
   Yes, users can either save the metadata to disk, or get the metadata object 
directly. There is a way that does not require this: embed the metadata to the 
end of the FST bytes, but that would be a bit too radical?



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] LUCENE-10195: Improve Gradle build speed [lucene]

2023-11-04 Thread via GitHub



Dawid commented on PR #414:
URL: https://github.com/apache/lucene/pull/414#issuecomment-1793567156

   > This is the problem with github at mentions, @mikemccand - whoever this is 
that had to drive you home, it wasn't me...
   
   Dawid, please don't treat it as problem, but as a miracle/opportunity. Mike 
from the other side of the Atlantic Ocean (6300km from us!) just summoned me 
mistakenly, but... we are both named Dawid, we are both from Poznan, we both 
bike, we are both in our 40-ties. You are located at Bóżnicza Street (or at 
least your wife is), I'm located at Mostowa Street which is few hundred meters 
away. I bet we both curse ZDM for recent Garbary/Małe Garbary road works. I 
even Google-street-viewed Bóżnicza earlier today to check how the new Stara 
Rzeźnia will look from this side. We also have few common friends from Allegro.
   
   That'a crazy Mike what you did here. ;) 


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] LUCENE-10195: Improve Gradle build speed [lucene]

2023-11-04 Thread via GitHub



dweiss commented on PR #414:
URL: https://github.com/apache/lucene/pull/414#issuecomment-1793561529

   Anyway - it's going to be difficult to saturate your CPU with tests alone, 
especially on a beefy machine. We limit the number of forked test JVMs - this 
you could tweak - but you'll soon hit memory bandwidth and I/O bandwidth limits 
(sooner than CPU usage limits, I think).
   
   This said, sure - there's room for improvement. For example, we used to have 
test balancing (reordering to try to saturate forked jvm queues) - I don't 
think this is possible with gradle's public APIs. There's also more interesting 
outstanding issues that never got solved, like this one that was closed and 
swept under the rug, but in fact is still an issue:
   
   https://github.com/gradle/gradle/issues/11609#issuecomment-910242430
   


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] LUCENE-10195: Improve Gradle build speed [lucene]

2023-11-04 Thread via GitHub



dweiss commented on PR #414:
URL: https://github.com/apache/lucene/pull/414#issuecomment-1793560062

   This is the problem with github at mentions, @mikemccand - whoever this is 
that had to drive you home, it wasn't me... 


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



rmuir commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793551757

   and yeah, the `avx-turbo` is measuring double precision when it "benches" 
FMA and we do float precision, i know. but its code already written and a nice 
non-java way to get the wanted info, and seems pretty in line with the results.


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



rmuir commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793549914

   for transparency, this was my testing procedure. I did lots of other things 
such as poking around and running experiments too, but for the basics of 
"running benchmark across different instance types", it can all be easily 
automated with tools like ansible and run all in parallel. the question is, how 
to visualize the data?
   ```
   # login
   ssh -i robkeypair.pem ec2-user@
   # disable system slowdowns
   sudo grubby --remove-args="selinux=1 security=selinux quiet" 
--args="mitigations=0 random.trust_cpu=1 loglevel=7 selinux=0" 
--update-kernel=ALL && sudo reboot
   # login again
   ssh -i robkeypair.pem ec2-user@
   -if x86
 # install packages for testing
 sudo yum install -y git g++ make
 # clone avx-turbo
 git clone g...@github.com:travisdowns/avx-turbo.git
 # build avx-turbo
 cd avx-turbo; make
 # load msr module
 sudo modprobe msr
 # run avx-turbo
 sudo ./avx-turbo
 # examine results, look for any oddities
   # look at avx*_imul, avx*_fma, and avx*_fma_t.
   # check ratio of avx512_imul to avx256_imul and look at clock difference
   # check ratio of avx512_fma_t to avx256_fma_t and look at clock 
difference
   # check ratio of avx*_fma_t to avx*_fma (divided by 2 for HT)
 cd ..
 curl -f 
https://download.java.net/java/GA/jdk21.0.1/415e3f918a1f4062a0074a2794853d0d/12/GPL/openjdk-21.0.1_linux-x64_bin.tar.gz
 | tar -zxvf -
   -else aarch64
 sudo yum install -y git
 curl -f 
https://download.java.net/java/GA/jdk21.0.1/415e3f918a1f4062a0074a2794853d0d/12/GPL/openjdk-21.0.1_linux-aarch64_bin.tar.gz
 | tar -zxvf -
   -endif
   # download java
   # configure java (also in case i get disconnected)
   echo 'export JAVA_HOME=/home/ec2-user/jdk-21.0.1' >> ~/.bashrc
   echo 'export PATH=$JAVA_HOME/bin:$PATH' >> ~/.bashrc
   source ~/.bashrc
   # prevent benchmark interference from daemon
   mkdir ~/.gradle
   echo 'org.gradle.daemon=false' > ~/.gradle/gradle.properties
   # clone lucene
   git clone g...@github.com:rmuir/lucene.git; cd lucene
   # run benchmark (main)
   ./gradlew -p lucene/benchmark-jmh assemble
   java -jar 
lucene/benchmark-jmh/build/benchmarks/lucene-benchmark-jmh-10.0.0-SNAPSHOT.jar 
float -p size=1024
   # run benchmark (patch)
   git checkout float_scalar_fma_unroll
   ./gradlew -p lucene/benchmark-jmh assemble
   java -jar 
lucene/benchmark-jmh/build/benchmarks/lucene-benchmark-jmh-10.0.0-SNAPSHOT.jar 
float -p size=1024
   ```


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Fix a bug in ShapeTestUtil [lucene]

2023-11-04 Thread via GitHub



stefanvodita commented on PR #12287:
URL: https://github.com/apache/lucene/pull/12287#issuecomment-1793516615

   The test could call the modified methods with a random `box` and assert that 
all vertices of the given polygon are different. We can reuse 
`hasIdenticalVertices` from #12757.


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



rmuir commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793512513

   > It is good that we have the sysprops to enforce FMA or disable it, 
overriding default detection if needed. So on apple chips with Linux you can 
disable it. 
   
   exactly. we can't detect all cases perfectly or predict the future. but I 
don't want this to be a hassle: and want things to be fast by default 
everywhere if at all possible (without complex logic). Hence the simple 
heuristic. If there is a problem with it, there's a workaround (sysprop).


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] LUCENE-10195: Improve Gradle build speed [lucene]

2023-11-04 Thread via GitHub



Dawid commented on PR #414:
URL: https://github.com/apache/lucene/pull/414#issuecomment-1793499490

   TLDR;
   No problemo, Mickey. You can always count on me, just like last Friday when 
we all get wasted and I had to Uber you home. Take care!


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Random access term dictionary [lucene]

2023-11-04 Thread via GitHub

nknize commented on code in PR #12688:
URL: https://github.com/apache/lucene/pull/12688#discussion_r1382429884

##
lucene/sandbox/src/java/org/apache/lucene/sandbox/codecs/lucene90/randomaccess/bitpacking/BitPacker.java:
##

Review Comment:
> Since they are of the same size...

That's the difference. In your use case the records (blocks) are guaranteed
to be the same size where as in the serialized tree case the records (tree
nodes) are not guaranteed to be the same size. This is by design to ensure the
resulting docvalue disk consumption is as efficient (small) as possible.

> ...by a quick glance it seems to me it encodes values with variable length
(VInt, VLong). Maybe the random-access is achieved in different ways?

Yes to variable length encoding. The "random-ness" isn't purely random in
that traversal of the serialized tree is DFS. Because the tree nodes are
variable size the serialized array includes copious "book-keeping" in the form
of "sizeOf" values. At DFS traversal the first "sizeOf" value provides the size
of the entire left tree. To prune the left tree just means we skip that many
bytes to get to the right tree.. this continues recursively. In practice we
don't expect to ever "back up" in our DFS traversal so there is only a `rewind`
method that simply resets the offset values to 0.

Seems the two use cases are subtly different but I could see roughly 80%
overlap in the implementation. I'd love to efficiently encapsulate this logic
for the next contributor that wants a random serialized traversal mechanism
without a ridiculous amount of java object overhead. Sounds like
@bruno-roustant had the same need? Could be a good follow on progress PR.

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

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

-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



uschindler commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793489975

   > You may not like my detector, but I think it is quite practical and 
prevents slow execution.
   
   The detector is funny, but it won't detect slow apple silicon if you run 
Linux on the Mac. But I agree it is ok.
   
   It is good that we have the sysprops to enforce FMA or disable it, 
overriding default detection if needed. So on apple chips with Linux you can 
disable it. 


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



rmuir commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793488056

   Here are the ARMs. I had to tweak ARM to use FMA more aggressively to fully 
utilize the gravitons. The problem there is just apple silicon, it is good we 
did not move forwards with benchmarks based solely on some macs. You may not 
like my detector, but I think it is quite practical and prevents slow execution.
   
   Graviton 3
   ```
   Main:
   Benchmark  (size)   Mode  Cnt   Score
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   0.682 ±  
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   5.500 ±  
0.004  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   2.411 ±  
0.037  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  11.522 ±  
0.234  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   2.169 ±  
0.005  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75   8.632 ±  
0.084  ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.422 ± 
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   6.911 ± 
0.039  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.751 ± 
0.007  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  11.498 ± 
0.418  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   3.202 ± 
0.007  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  10.795 ± 
0.154  ops/us
   ```
   
   
   Graviton 2
   ```
   Main:
   Benchmark  (size)   Mode  Cnt  Score   Error 
  Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15  0.647 ± 0.002 
 ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75  2.599 ± 0.002 
 ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15  1.430 ± 0.007 
 ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  6.192 ± 0.098 
 ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15  1.194 ± 0.003 
 ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  4.797 ± 0.088 
 ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt  Score
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15  1.571 ±  
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75  5.408 ±  
0.013  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15  2.055 ±  
0.066  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  6.673 ±  
0.260  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15  1.753 ±  
0.001  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  6.179 ±  
0.070  ops/us
   ```
   
   Mac M1
   ```
   Main:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.077 ± 
0.002  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   7.651 ± 
0.032  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.606 ± 
0.032  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  16.296 ± 
0.268  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   3.197 ± 
0.001  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  14.185 ± 
0.099  ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   2.062 ± 
0.006  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   7.644 ± 
0.030  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   4.273 ± 
0.003  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  16.110 ± 
0.283  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   3.770 ± 
0.007  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  14.184 ± 
0.100  ops/us
   ```


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [I] Should we not enlarge PagedGrowableWriter initial bitPerValue on NodeHash.rehash()? [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on issue #12744:
URL: https://github.com/apache/lucene/issues/12744#issuecomment-1793487558

   > Does that mean every values, including the ones with low-address will use 
the same bpv as the high-address nodes? PagedGrowableWriter already enlarge the 
bpv 
[automatically](https://github.com/apache/lucene/blob/8fa0de2743e87dd264619632978c8dc68323947a/lucene/core/src/java/org/apache/lucene/util/packed/GrowableWriter.java#L86),
 so maybe we could always set the initial bpv to a small value (like 8?)
   
   OK so I think normally `PagedGrowableWriter` would indeed be more compact if 
earlier values used smaller bpv than later values, *and* the paging was small 
enough so that there were enough pages to have separate mutables with different 
`bpv`?
   
   But in our usage here as a hash table, the inputs will more or less randomly 
scatter across the pages to the point where all pages will soon have to update 
to the large (current) bpv on insert anyways?


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [I] Should we not enlarge PagedGrowableWriter initial bitPerValue on NodeHash.rehash()? [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on issue #12744:
URL: https://github.com/apache/lucene/issues/12744#issuecomment-1793486887

   > I think this should be enhancement instead of bug, but I can't edit it. 
@mikemccand can you help to change the label?
   
   Done.  Annoying that GH won't let you do that, especially when you are the 
one who opened it!


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Make FST fully read-only and streamline FST constructor [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on PR #12691:
URL: https://github.com/apache/lucene/pull/12691#issuecomment-1793486552

   We closed this PR in favor of #12758?


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] LUCENE-10195: Improve Gradle build speed [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on PR #414:
URL: https://github.com/apache/lucene/pull/414#issuecomment-1793485933

   > > I don't like how slow our gradle builds are, so if we can make it 
faster, that'd be awesome.
   > 
   > Are they? What in particular is slow for you, Mike? There's tons of stuff 
that runs on (full) check across all submodules, for example. I don't think 
it's avoidable. In majority of cases, you can narrow down the task to a 
subproject and it runs very quickly for me.
   
   Well, maybe I am too impatient.  I don't have hard data to prove they are 
slow.  But if I watch `top` while running toplevel `./gradlew test` I am 
disappointed at how much concurrency is left on the table.
   
   I do see lots of concurrency (many `java` processes) when the actual unit 
testing seems to start, which is great.
   
   With my `lucene` clone already fully compiled, the gradle build seems to 
take a few (~3) seconds to confirm everything is up-to-date, printing out lines 
like `> Task :lucene:benchmark:jar UP-TO-DATE  `.  This is on a modern Raptor 
Lake Intel CPU.
   
   So yeah I retract my statement :)  It's not clearly slow.  `./gradlew test` 
finishes in 56s for me (on fully built clone).  I remember times in the past 
when this was much longer ... thanks @dawid and @clayburn.


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] LUCENE-10195: Improve Gradle build speed [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on PR #414:
URL: https://github.com/apache/lucene/pull/414#issuecomment-1793484726

   > @mikemccand - If you are interested, 
[ge.apache.org](https://ge.apache.org/scans?search.rootProjectNames=lucene-root=America%2FChicago)
 is available to the Lucene project. Currently, it is all CI builds, but you 
can opt in to this for your local builds as an ASF committer using your ASF 
LDAP credentials and [provisioning an access 
key](https://docs.gradle.com/enterprise/gradle-plugin/#automated_access_key_provisioning).
 If you did that and produced build scans for some of the slow builds you see, 
we would be happy to offer advice.
   
   Whoa, I did not even know about this service!  Thanks for the pointer 
@clayburn 


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Streamline FST constructors and make it fully read-only [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on code in PR #12758:
URL: https://github.com/apache/lucene/pull/12758#discussion_r1382420527


##
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##
@@ -411,17 +401,42 @@ public FST(DataInput metaIn, DataInput in, Outputs 
outputs) throws IOExceptio
 this(metaIn, in, outputs, new OnHeapFSTStore(DEFAULT_MAX_BLOCK_BITS));
   }
 
+  /** Load a previously saved FST. */
+  public FST(DataInput metaIn, DataInput in, Outputs outputs, FSTStore 
fstStore)
+  throws IOException {
+this(parseMetadata(metaIn, outputs), in, outputs, fstStore);
+  }
+
   /**
* Load a previously saved FST; maxBlockBits allows you to control the size 
of the byte[] pages
* used to hold the FST bytes.
*/
-  public FST(DataInput metaIn, DataInput in, Outputs outputs, FSTStore 
fstStore)
+  public FST(FSTMetadata metadata, DataInput in, Outputs outputs, 
FSTStore fstStore)
   throws IOException {
-this.outputs = outputs;
+this(metadata, outputs, initStore(fstStore, metadata.numBytes, in));
+  }
+
+  private static FSTReader initStore(FSTStore fstStore, long numBytes, 
DataInput in)
+  throws IOException {
+fstStore.init(in, numBytes);
+return fstStore;
+  }
 
+  /**
+   * Parse the FST metadata from DataInput
+   *
+   * @param metaIn the DataInput of the metadata
+   * @param outputs the FST outputs
+   * @return the FST metadata
+   * @param  the output type
+   * @throws IOException if exception occurred during parsing
+   */
+  public static  FSTMetadata parseMetadata(DataInput metaIn, Outputs 
outputs)

Review Comment:
   Maybe `readMetadata` since it is reading from a previously serialized 
`DataInput` source?



##
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##
@@ -1132,4 +1137,28 @@ public abstract static class BytesReader extends 
DataInput {
 /** Returns true if this reader uses reversed bytes under-the-hood. */
 public abstract boolean reversed();
   }
+
+  /**
+   * Represent the FST metadata
+   *
+   * @param  the FST output type
+   */
+  public static class FSTMetadata {
+INPUT_TYPE inputType;
+// if non-null, this FST accepts the empty string and
+// produces this output
+T emptyOutput;

Review Comment:
   Can all of these members be `final`?
   
   Edit: no they cannot :)



##
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##
@@ -1132,4 +1137,28 @@ public abstract static class BytesReader extends 
DataInput {
 /** Returns true if this reader uses reversed bytes under-the-hood. */
 public abstract boolean reversed();
   }
+
+  /**
+   * Represent the FST metadata
+   *
+   * @param  the FST output type
+   */
+  public static class FSTMetadata {

Review Comment:
   Can the class be `final`?
   
   This is public because callers would need to separately write/read the 
`FSTMetadata` some their sources?  The FST bytes can stream append only to 
disk, but the end, the compiler produces the final `FSTMetadata` which must 
then be stored somewhere?



##
lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java:
##
@@ -828,6 +829,26 @@ public void add(IntsRef input, T output) throws 
IOException {
 lastInput.copyInts(input);
   }
 
+  void setEmptyOutput(T v) {
+if (fst.metadata.emptyOutput != null) {
+  fst.metadata.emptyOutput = fst.outputs.merge(fst.metadata.emptyOutput, 
v);

Review Comment:
   Ahh OK `emptyOutput` cannot be final!



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Streamline FST constructors and make it fully read-only [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on PR #12758:
URL: https://github.com/apache/lucene/pull/12758#issuecomment-1793483247

   > Note: We also might want to remove the constructor with FSTStore 
completely, and users need to call `init()` themselves?
   
   +1


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Specialize arc store for continuous label in FST [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on code in PR #12748:
URL: https://github.com/apache/lucene/pull/12748#discussion_r1382419312


##
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##
@@ -96,6 +96,13 @@ public enum INPUT_TYPE {
*/
   static final byte ARCS_FOR_DIRECT_ADDRESSING = 1 << 6;
 
+  /**
+   * Value of the arc flags to declare a node with continuous arcs designed 
for pos the arc directly
+   * with labelToPos - firstLabel. like {@link #ARCS_FOR_BINARY_SEARCH} we use 
flag combinations
+   * that will not occur at the same time.
+   */
+  static final byte ARCS_FOR_CONTINUOUS = ARCS_FOR_DIRECT_ADDRESSING + 
ARCS_FOR_BINARY_SEARCH;
+
   // Increment version to change it
   private static final String FILE_FORMAT_NAME = "FST";
   private static final int VERSION_START = 6;

Review Comment:
   Hmm shouldn't we bump the `VERSION_CURRENT` in FST with this change too?



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Specialize arc store for continuous label in FST [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on PR #12748:
URL: https://github.com/apache/lucene/pull/12748#issuecomment-1793477236

   Hello @easyice, I'm sorry but I just merged #12738 which caused conflicts 
here ... could you please rebase and resolve conflicts?  I think this change is 
ready except for that.  Thank you!!


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on PR #12738:
URL: https://github.com/apache/lucene/pull/12738#issuecomment-1793476420

   I merged to main, thank you @dungba88 for the fast iterations!  I could 
barely keep up just reviewing :)
   
   After all this FST dust settles let's remember to add your CHANGES.txt entry 
summarizing all the progress with capping RAM usage of FSTs.  I think we can 
make one entry (something like "FST Compiler can now build arbitrary large FSTs 
with capped/controllable RAM usage"), linking to the N GitHub issues/PRs it 
took to accomplish.  Maybe after we backport to 9.x?


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [I] FSTCompiler's NodeHash should fully duplicate `byte[]` slices from the growing FST [lucene]

2023-11-04 Thread via GitHub



mikemccand closed issue #12714: FSTCompiler's NodeHash should fully duplicate 
`byte[]` slices from the growing FST
URL: https://github.com/apache/lucene/issues/12714


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

2023-11-04 Thread via GitHub



mikemccand merged PR #12738:
URL: https://github.com/apache/lucene/pull/12738


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

[PR] Remove usage of deprecated java.util.Locale constructor [lucene]

2023-11-04 Thread via GitHub



ChrisHegarty opened a new pull request, #12761:
URL: https://github.com/apache/lucene/pull/12761

   This commit removes usages of the deprecated `java.util.Locale` constructor 
with `Locale.Builder`.
   
   The motivation for this change is to address tech debt identified when 
experimenting with bumping to Java 21.
   
   The changes in this PR are, for the most part, from @rmuir. Extracted out of 
#12753.
   
   relates #12753


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



rmuir commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793456342

   Benchmarks for the intel cpus. There is one place i'd fix, if we could 
detect sapphire rapids and avoid scalar FMA. But i have no way to detect it 
based on what new features it has / what openjdk exposes at the moment. 
Otherwise performance is good.
   
   Sapphire Rapids:
   ```
   Main:
   Benchmark   (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar 1024  thrpt   15   0.871 ± 
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector 1024  thrpt   75  13.907 ± 
0.266  ops/us
   VectorUtilBenchmark.floatDotProductScalar 1024  thrpt   15   4.275 ± 
0.023  ops/us
   VectorUtilBenchmark.floatDotProductVector 1024  thrpt   75  22.218 ± 
0.759  ops/us
   VectorUtilBenchmark.floatSquareScalar 1024  thrpt   15   2.819 ± 
0.004  ops/us
   VectorUtilBenchmark.floatSquareVector 1024  thrpt   75  20.243 ± 
0.352  ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.650 ± 
0.002  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75  13.799 ± 
0.233  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.612 ± 
0.012  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  23.300 ± 
1.079  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   2.884 ± 
0.004  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  20.449 ± 
0.446  ops/us
   ```
   
   Ice Lake:
   ```
   Main:
   Benchmark   (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar 1024  thrpt   15   0.547 ± 
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector 1024  thrpt   75   9.842 ± 
0.334  ops/us
   VectorUtilBenchmark.floatDotProductScalar 1024  thrpt   15   2.471 ± 
0.002  ops/us
   VectorUtilBenchmark.floatDotProductVector 1024  thrpt   75  13.452 ± 
0.455  ops/us
   VectorUtilBenchmark.floatSquareScalar 1024  thrpt   15   1.749 ± 
0.004  ops/us
   VectorUtilBenchmark.floatSquareVector 1024  thrpt   75  11.813 ± 
0.456  ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.528 ± 
0.003  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   9.919 ± 
0.345  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.314 ± 
0.003  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  13.137 ± 
0.155  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   3.248 ± 
0.025  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  11.920 ± 
0.469  ops/us
   ```
   
   Cascade Lake:
   ```
   Main:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   0.578 ± 
0.005  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   8.907 ± 
0.095  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   1.742 ± 
0.003  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  13.935 ± 
0.129  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   1.347 ± 
0.005  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  12.526 ± 
0.132  ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.641 ± 
0.002  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   8.823 ± 
0.114  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.401 ± 
0.014  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  13.874 ± 
0.116  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   2.629 ± 
0.016  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  12.462 ± 
0.123  ops/us
   ```
   
   Haswell:
   ```
   Main:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   0.728 ± 
0.005  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   6.781 ± 
0.071  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   1.730 ± 
0.034  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  10.603 ± 
0.351  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   1.398 ± 
0.060  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75   9.470 ± 
0.286  ops/us

Re: [I] Add Scalar Quantization codec for Vectors [lucene]

2023-11-04 Thread via GitHub



benwtrent commented on issue #12497:
URL: https://github.com/apache/lucene/issues/12497#issuecomment-1793446854

   I have done a poor job of linking against the related work for bringing 
vector lossy-compression.
   
   The initial implementation of adding int8 (really, its int7 because of 
signed bytes...): https://github.com/apache/lucene/pull/12582
   
   A significant refactor to make adding new quantized storage easier: 
https://github.com/apache/lucene/pull/12729
   
   


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] TestIndexWriterOnVMError.testUnknownError times out (fixes potential IW deadlock on tragic exceptions). [lucene]

2023-11-04 Thread via GitHub



s1monw commented on code in PR #12751:
URL: https://github.com/apache/lucene/pull/12751#discussion_r1382391250


##
lucene/core/src/java/org/apache/lucene/index/IndexWriter.java:
##
@@ -2560,10 +2560,15 @@ private void rollbackInternalNoCommit() throws 
IOException {
 
   // close all the closeables we can (but important is readerPool and 
writeLock to prevent
   // leaks)
-  IOUtils.closeWhileHandlingException(readerPool, deleter, writeLock);
-  writeLock = null;
-  closed = true;
-  closing = false;
+  try {
+IOUtils.closeWhileHandlingException(readerPool, deleter, 
writeLock);
+  } catch (Throwable t) {
+throwable.addSuppressed(t);
+  } finally {
+writeLock = null;
+closed = true;
+closing = false;
+  }
 
   // So any "concurrently closing" threads wake up and see that the 
close has now completed:
   notifyAll();

Review Comment:
   yeah you are right, this is also a recipe for deadlocks here... maybe we 
should do something like this:
   
   ```diff
   --- a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
   +++ b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
   @@ -2463,7 +2463,15 @@ public class IndexWriter
if (infoStream.isEnabled("IW")) {
  infoStream.message("IW", "rollback");
}
   -
   +Closeable cleanupAndNotify =
   +() -> {
   +  assert Thread.holdsLock(this);
   +  writeLock = null;
   +  closed = true;
   +  closing = false;
   +  // So any "concurrently closing" threads wake up and see that the 
close has now completed:
   +  notifyAll();
   +};
try {
  synchronized (this) {
// must be synced otherwise register merge might throw and 
exception if merges
   @@ -2531,43 +2539,35 @@ public class IndexWriter
// after we leave this sync block and before we enter the sync 
block in the finally clause
// below that sets closed:
closed = true;
   -
   -IOUtils.close(writeLock); // release write lock
   -writeLock = null;
   -closed = true;
   -closing = false;
   -// So any "concurrently closing" threads wake up and see that the 
close has now completed:
   -notifyAll();
   +IOUtils.close(writeLock, cleanupAndNotify); // release write lock
  }
} catch (Throwable throwable) {
  try {
// Must not hold IW's lock while closing
// mergeScheduler: this can lead to deadlock,
// e.g. TestIW.testThreadInterruptDeadlock
   -IOUtils.closeWhileHandlingException(mergeScheduler);
   -synchronized (this) {
   -  // we tried to be nice about it: do the minimum
   -  // don't leak a segments_N file if there is a pending commit
   -  if (pendingCommit != null) {
   -try {
   -  pendingCommit.rollbackCommit(directory);
   -  deleter.decRef(pendingCommit);
   -} catch (Throwable t) {
   -  throwable.addSuppressed(t);
   -}
   -pendingCommit = null;
   -  }
   -
   -  // close all the closeables we can (but important is readerPool 
and writeLock to prevent
   -  // leaks)
   -  IOUtils.closeWhileHandlingException(readerPool, deleter, 
writeLock);
   -  writeLock = null;
   -  closed = true;
   -  closing = false;
   -
   -  // So any "concurrently closing" threads wake up and see that the 
close has now completed:
   -  notifyAll();
   -}
   +IOUtils.closeWhileHandlingException(
   +mergeScheduler,
   +() -> {
   +  synchronized (this) {
   +// we tried to be nice about it: do the minimum
   +// don't leak a segments_N file if there is a pending commit
   +if (pendingCommit != null) {
   +  try {
   +pendingCommit.rollbackCommit(directory);
   +deleter.decRef(pendingCommit);
   +  } catch (Throwable t) {
   +throwable.addSuppressed(t);
   +  }
   +  pendingCommit = null;
   +}
   +// close all the closeables we can (but important is 
readerPool and writeLock to
   +// prevent
   +// leaks)
   +IOUtils.closeWhileHandlingException(
   +readerPool, deleter, writeLock, cleanupAndNotify);
   +  }
   +});
  } catch (Throwable t) {
throwable.addSuppressed(t);
  } finally {
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

2023-11-04 Thread via GitHub



dungba88 commented on code in PR #12738:
URL: https://github.com/apache/lucene/pull/12738#discussion_r1382384038


##
lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java:
##
@@ -38,6 +38,8 @@ public final class ByteBlockPool implements Accountable {
 
   /** Abstract class for allocating and freeing byte blocks. */
   public abstract static class Allocator {
+// TODO: ByteBlockPool assume the blockSize is always {@link 
BYTE_BLOCK_SIZE}, but this class
+// allow arbitrary value of blockSize. We should make them consistent.
 protected final int blockSize;

Review Comment:
   We don't have, and this Allocator seems to be only used by ByteBlockPool so 
maybe we don't need it to have custom block size?



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

[I] Improve bytes copy in NodeHash [lucene]

2023-11-04 Thread via GitHub



dungba88 opened a new issue, #12760:
URL: https://github.com/apache/lucene/issues/12760

   ### Description
   
   Spawn of https://github.com/apache/lucene/pull/12738, there are 2 TODOs 
about reducing byte copies when copying from FST and when promoting from the 
fallback table.


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Streamline FST constructors and make it fully read-only [lucene]

2023-11-04 Thread via GitHub



dungba88 commented on code in PR #12758:
URL: https://github.com/apache/lucene/pull/12758#discussion_r1382383218


##
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##
@@ -411,17 +401,42 @@ public FST(DataInput metaIn, DataInput in, Outputs 
outputs) throws IOExceptio
 this(metaIn, in, outputs, new OnHeapFSTStore(DEFAULT_MAX_BLOCK_BITS));
   }
 
+  /** Load a previously saved FST. */
+  public FST(DataInput metaIn, DataInput in, Outputs outputs, FSTStore 
fstStore)
+  throws IOException {
+this(parseMetadata(metaIn, outputs), in, outputs, fstStore);
+  }
+
   /**

Review Comment:
   The Javadoc could be outdated. Will update it



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



rmuir commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793426911

   > @rmuir: It would be nice if you could follow the community standard and 
merge this long PR with Github UI and squash it - thanks. I can do it for you 
if you like.
   
   I am not done here yet, I want to benchmark and try to tighten the intel and 
arm models first too. At least do the best i can to get the best performance 
out of all of them.
   
   whether to squash or not is my decision. Just like maybe the community 
standard is intellij, i use vim.


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



uschindler commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793426843

   > I tested on my now-ancient Zen2 beast3 (nightly benchmark) box (`AMD Ryzen 
Threadripper 3990X 64-Core Processor`), using JDK 21 (`openjdk full version 
"21+35"`), with command-line `./gradlew clean; ./gradlew -p 
lucene/benchmark-jmh assemble; java -jar 
lucene/benchmark-jmh/build/benchmarks/lucene-benchmark-jmh-10.0.0-SNAPSHOT.jar 
float -p size=1024`.
   > 
   > [An aside: strangely, to test the PR, I normally download and apply the 
[`.diff`](https://patch-diff.githubusercontent.com/raw/apache/lucene/pull/12737.diff)
 or 
[`.patch`](https://patch-diff.githubusercontent.com/raw/apache/lucene/pull/12737.patch)
 using `patch -p1 < X.diff/patch`, but for this PR there are non-trivial (to 
me!) conflicts reported by `patch`. So instead I ran the suggested `github` 
command-line steps for merging, and got a clean applied version of this PR to 
run the benchy.]
   > 
   > `main`:
   > 
   > ```
   > Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   > VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.176 ± 
0.011  ops/us
   > VectorUtilBenchmark.floatCosineVector1024  thrpt   75  11.015 ± 
0.029  ops/us
   > VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.870 ± 
0.011  ops/us
   > VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  22.879 ± 
0.407  ops/us
   > VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   2.604 ± 
0.023  ops/us
   > VectorUtilBenchmark.floatSquareVector1024  thrpt   75  21.293 ± 
0.289  ops/us
   > ```
   > 
   > PR:
   > 
   > ```
   > Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   > VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.553 ± 
0.009  ops/us
   > VectorUtilBenchmark.floatCosineVector1024  thrpt   75  10.995 ± 
0.025  ops/us
   > VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   4.051 ± 
0.029  ops/us
   > VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  22.887 ± 
0.396  ops/us
   > VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   3.254 ± 
0.008  ops/us
   > VectorUtilBenchmark.floatSquareVector1024  thrpt   75  21.238 ± 
0.420  ops/us
   > ```
   
   There are some bugs in Github since yesterday (they also 404 not found PRs 
for some time). Actually the patch is completely unuseable as it partly 
contains also merged information. The diff looks fine to me.
   
   My recommendation: You can merge the PR into your branch using the command 
line provided by Github or - much easier - add Robert's repository as rmuir 
upsteam. I have the common repos by Robert, Chris already available in my git 
config, so it's simple to check them out and work directly on them.


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on code in PR #12738:
URL: https://github.com/apache/lucene/pull/12738#discussion_r1382379237


##
lucene/core/src/test/org/apache/lucene/util/TestByteBlockPool.java:
##
@@ -91,6 +92,10 @@ public void testLargeRandomBlocks() throws IOException {
   random().nextBytes(bytes);
   items.add(bytes);
   pool.append(new BytesRef(bytes));
+  totalBytes += size;
+
+  // make sure we report the correct position
+  assertEquals(totalBytes, pool.getPosition());

Review Comment:
   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...@lucene.apache.org

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on code in PR #12738:
URL: https://github.com/apache/lucene/pull/12738#discussion_r1382379017


##
lucene/core/src/java/org/apache/lucene/util/ByteBlockPool.java:
##
@@ -38,6 +38,8 @@ public final class ByteBlockPool implements Accountable {
 
   /** Abstract class for allocating and freeing byte blocks. */
   public abstract static class Allocator {
+// TODO: ByteBlockPool assume the blockSize is always {@link 
BYTE_BLOCK_SIZE}, but this class
+// allow arbitrary value of blockSize. We should make them consistent.
 protected final int blockSize;

Review Comment:
   Hmm do we have any `if` or `assert` that confirms `Allocator`'s `blockSize` 
== `ByteBlockPool.BYTE_BLOCK_SIZE` when passed to `ByteBlockPool`?



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

[I] Remove `FST.BytesReader#reversed` method? [lucene]

2023-11-04 Thread via GitHub



mikemccand opened a new issue, #12759:
URL: https://github.com/apache/lucene/issues/12759

   ### Description
   
   Spinoff from #12738: this method seems to be dead/pointless code now?


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Use value-based LRU cache in NodeHash [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on code in PR #12738:
URL: https://github.com/apache/lucene/pull/12738#discussion_r1382378735


##
lucene/core/src/java/org/apache/lucene/util/fst/ByteBlockPoolReverseBytesReader.java:
##
@@ -0,0 +1,69 @@
+/*
+ * 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.lucene.util.fst;
+
+import java.io.IOException;
+import org.apache.lucene.util.ByteBlockPool;
+
+/** Reads in reverse from a ByteBlockPool. */
+final class ByteBlockPoolReverseBytesReader extends FST.BytesReader {
+
+  private final ByteBlockPool buf;
+  // the difference between the FST node address and the hash table copied 
node address
+  private long posDelta;
+  private long pos;
+
+  public ByteBlockPoolReverseBytesReader(ByteBlockPool buf) {
+this.buf = buf;
+  }
+
+  @Override
+  public byte readByte() {
+return buf.readByte(pos--);
+  }
+
+  @Override
+  public void readBytes(byte[] b, int offset, int len) {
+for (int i = 0; i < len; i++) {
+  b[offset + i] = buf.readByte(pos--);
+}
+  }
+
+  @Override
+  public void skipBytes(long numBytes) throws IOException {
+pos -= numBytes;
+  }
+
+  @Override
+  public long getPosition() {
+return pos + posDelta;
+  }
+
+  @Override
+  public void setPosition(long pos) {
+this.pos = pos - posDelta;
+  }
+
+  @Override
+  public boolean reversed() {

Review Comment:
   I'll open a spinoff issue for this -- it seems at quick glance to be 
dead/pointless code.



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793421290

   I tested on my now-ancient Zen2 beast3 (nightly benchmark) box (`AMD Ryzen 
Threadripper 3990X 64-Core Processor`), using JDK 21 (`openjdk full version 
"21+35"`), with command-line `./gradlew clean; ./gradlew -p 
lucene/benchmark-jmh assemble; java -jar 
lucene/benchmark-jmh/build/benchmarks/lucene-benchmark-jmh-10.0.0-SNAPSHOT.jar 
float -p size=1024`.
   
   [An aside: strangely, to test the PR, I normally download and apply the 
[`.diff`](https://patch-diff.githubusercontent.com/raw/apache/lucene/pull/12737.diff)
 or 
[`.patch`](https://patch-diff.githubusercontent.com/raw/apache/lucene/pull/12737.patch)
 using `patch -p1 < X.diff/patch`, but for this PR there are non-trivial (to 
me!) conflicts reported by `patch`.  So instead I ran the suggested `github` 
command-line steps for merging, and got a clean applied version of this PR to 
run the benchy.]
   
   `main`:
   
   ```
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.176 ± 
0.011  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75  11.015 ± 
0.029  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.870 ± 
0.011  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  22.879 ± 
0.407  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   2.604 ± 
0.023  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  21.293 ± 
0.289  ops/us
   ```
   
   PR:
   
   ```
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.553 ± 
0.009  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75  10.995 ± 
0.025  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   4.051 ± 
0.029  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  22.887 ± 
0.396  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   3.254 ± 
0.008  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  21.238 ± 
0.420  ops/us
   ```


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



uschindler commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793421197

   @rmuir: It would be nice if you could merge this long PR with Github UI and 
squash it - thanks. I can do it for you if you like.


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



mikemccand commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793399807

   Thank you @rmuir for doing all the crazy hard work to decode the actual 
capabilities of the bare metal hiding underneath the layers of abstraction 
under Panama Vector API @rmuir!  I love the `CONSTANTS` approach.
   
   > versus intel's approach of slowing down other things on the computer :)
   
   !!


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Streamline FST constructors and make it fully read-only [lucene]

2023-11-04 Thread via GitHub



dungba88 commented on code in PR #12758:
URL: https://github.com/apache/lucene/pull/12758#discussion_r1382359394


##
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##
@@ -411,17 +401,42 @@ public FST(DataInput metaIn, DataInput in, Outputs 
outputs) throws IOExceptio
 this(metaIn, in, outputs, new OnHeapFSTStore(DEFAULT_MAX_BLOCK_BITS));
   }
 
+  /** Load a previously saved FST. */
+  public FST(DataInput metaIn, DataInput in, Outputs outputs, FSTStore 
fstStore)
+  throws IOException {
+this(parseMetadata(metaIn, outputs), in, outputs, fstStore);
+  }
+
   /**
* Load a previously saved FST; maxBlockBits allows you to control the size 
of the byte[] pages
* used to hold the FST bytes.
*/
-  public FST(DataInput metaIn, DataInput in, Outputs outputs, FSTStore 
fstStore)
+  public FST(FSTMetadata metadata, DataInput in, Outputs outputs, 
FSTStore fstStore)
   throws IOException {
-this.outputs = outputs;
+this(metadata, outputs, initStore(fstStore, metadata.numBytes, in));
+  }
+
+  private static FSTReader initStore(FSTStore fstStore, long numBytes, 
DataInput in)
+  throws IOException {
+fstStore.init(in, numBytes);

Review Comment:
   This can also be syntactic sugar by making `init` return the `FSTReader`



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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

[PR] Streamline FST constructors and make it fully read-only [lucene]

2023-11-04 Thread via GitHub



dungba88 opened a new pull request, #12758:
URL: https://github.com/apache/lucene/pull/12758

   ### Description
   
   - Streamline FST constructors by grouping the medata attributes into 
FSTMetadata
   - Make it fully read-only by moving `finish()` and `setEmptyOutput` to 
FSTCompiler
   
   Eventually we would want to remove/deprecate the constructors with 
`DataInput metaIn`


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Make FST fully read-only and streamline FST constructor [lucene]

2023-11-04 Thread via GitHub



dungba88 closed pull request #12691: Make FST fully read-only and streamline 
FST constructor
URL: https://github.com/apache/lucene/pull/12691


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]

2023-11-04 Thread via GitHub



rmuir commented on PR #12737:
URL: https://github.com/apache/lucene/pull/12737#issuecomment-1793362865

   I tweaked the FMA logic for AMD cpus, to only avoid the high-latency scalar 
FMA where necessary. Should appease germans to get that extra ulp or whatever.
   
   sysprops default to "auto" so you can override however you want, without 
fear of involving BigDecimal :)
   
   I can test the intel and arm families in the same way and try to tighten it 
up tomorrow.
   
   AMD Zen4: EPYC 9R14 (family 0x19)
   ```
   Main:
   Benchmark  (size)   Mode  Cnt   Score
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   0.842 ±  
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75  13.497 ±  
0.171  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.540 ±  
0.002  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  16.441 ±  
0.424  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   2.540 ±  
0.008  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  16.655 ±  
0.575  ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.763 ± 
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75  13.477 ± 
0.168  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.583 ± 
0.003  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  16.438 ± 
0.493  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   3.560 ± 
0.009  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  15.778 ± 
0.114  ops/us
   ```
   
   AMD Zen3: EPYC 7R13 (family 0x19)
   ```
   Main:
   Benchmark   (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar 1024  thrpt   15   0.982 ± 
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector 1024  thrpt   75  10.476 ± 
0.026  ops/us
   VectorUtilBenchmark.floatDotProductScalar 1024  thrpt   15   3.246 ± 
0.015  ops/us
   VectorUtilBenchmark.floatDotProductVector 1024  thrpt   75  16.959 ± 
0.480  ops/us
   VectorUtilBenchmark.floatSquareScalar 1024  thrpt   15   2.298 ± 
0.010  ops/us
   VectorUtilBenchmark.floatSquareVector 1024  thrpt   75  16.342 ± 
0.508  ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.344 ± 
0.001  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75  10.445 ± 
0.048  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.405 ± 
0.006  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  16.486 ± 
0.374  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   2.995 ± 
0.002  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  16.374 ± 
0.462  ops/us
   ```
   
   AMD Zen2: EPYC 7R32 (family 0x17)
   ```
   Main:
   Benchmark   (size)   Mode  Cnt   Score
Error   Units
   VectorUtilBenchmark.floatCosineScalar 1024  thrpt   15   0.922 ±  
0.005  ops/us
   VectorUtilBenchmark.floatCosineVector 1024  thrpt   75   8.519 ±  
0.020  ops/us
   VectorUtilBenchmark.floatDotProductScalar 1024  thrpt   15   2.968 ±  
0.020  ops/us
   VectorUtilBenchmark.floatDotProductVector 1024  thrpt   75  15.950 ±  
0.486  ops/us
   VectorUtilBenchmark.floatSquareScalar 1024  thrpt   15   2.015 ±  
0.012  ops/us
   VectorUtilBenchmark.floatSquareVector 1024  thrpt   75  15.894 ±  
0.331  ops/us
   
   Patch:
   Benchmark  (size)   Mode  Cnt   Score   
Error   Units
   VectorUtilBenchmark.floatCosineScalar1024  thrpt   15   1.200 ± 
0.005  ops/us
   VectorUtilBenchmark.floatCosineVector1024  thrpt   75   8.520 ± 
0.018  ops/us
   VectorUtilBenchmark.floatDotProductScalar1024  thrpt   15   3.114 ± 
0.021  ops/us
   VectorUtilBenchmark.floatDotProductVector1024  thrpt   75  15.671 ± 
0.439  ops/us
   VectorUtilBenchmark.floatSquareScalar1024  thrpt   15   2.490 ± 
0.030  ops/us
   VectorUtilBenchmark.floatSquareVector1024  thrpt   75  15.189 ± 
0.170  ops/us
   ```


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

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


-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands,

50 matches

Mail list logo