Re: [PR] Make FST fully read-only and streamline FST constructor [lucene]
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
[PR] Streamline FST constructors and make it fully read-only [lucene]
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] Streamline FST constructors and make it fully read-only [lucene]
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
Re: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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]
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] Use value-based LRU cache in NodeHash [lucene]
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
[I] Remove `FST.BytesReader#reversed` method? [lucene]
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]
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
Re: [PR] Use value-based LRU cache in NodeHash [lucene]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] Streamline FST constructors and make it fully read-only [lucene]
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
[I] Improve bytes copy in NodeHash [lucene]
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] Use value-based LRU cache in NodeHash [lucene]
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
Re: [PR] TestIndexWriterOnVMError.testUnknownError times out (fixes potential IW deadlock on tragic exceptions). [lucene]
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 GitHu
Re: [I] Add Scalar Quantization codec for Vectors [lucene]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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 P
[PR] Remove usage of deprecated java.util.Locale constructor [lucene]
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] Use value-based LRU cache in NodeHash [lucene]
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
Re: [I] FSTCompiler's NodeHash should fully duplicate `byte[]` slices from the growing FST [lucene]
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]
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: [PR] Specialize arc store for continuous label in FST [lucene]
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] Specialize arc store for continuous label in FST [lucene]
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] Streamline FST constructors and make it fully read-only [lucene]
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] Streamline FST constructors and make it fully read-only [lucene]
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] LUCENE-10195: Improve Gradle build speed [lucene]
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&search.timeZoneId=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] LUCENE-10195: Improve Gradle build speed [lucene]
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] Make FST fully read-only and streamline FST constructor [lucene]
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: [I] Should we not enlarge PagedGrowableWriter initial bitPerValue on NodeHash.rehash()? [lucene]
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: [I] Should we not enlarge PagedGrowableWriter initial bitPerValue on NodeHash.rehash()? [lucene]
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: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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: [PR] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] Random access term dictionary [lucene]
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] LUCENE-10195: Improve Gradle build speed [lucene]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] Fix a bug in ShapeTestUtil [lucene]
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]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] LUCENE-10195: Improve Gradle build speed [lucene]
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] LUCENE-10195: Improve Gradle build speed [lucene]
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]
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] Streamline FST constructors and make it fully read-only [lucene]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] Make FST fully read-only and streamline FST constructor [lucene]
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] Streamline FST constructors and make it fully read-only [lucene]
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] Speed up vectorutil float scalar methods, unroll properly, use fma where possible [lucene]
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] Use value-based LRU cache in NodeHash [lucene]
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] Specialize arc store for continuous label in FST [lucene]
easyice commented on code in PR #12748:
URL: https://github.com/apache/lucene/pull/12748#discussion_r1382516071
##
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:
I think so, we expect to throw `IndexFormatTooNewException` in`
CodecUtil.checkIndexHeader` when the old version code reading new index format.
so we should also bump the `VERSION_CURRENT` in `Lucene90BlockTreeTermsReader`?
--
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
