[GitHub] [lucene] uschindler commented on a diff in pull request #11937: reland: 11925 javac options
uschindler commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023668563
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
I think as Robert said: The internals of javacompile do not execute "javac"
and instead use "java" together with some own code!?
I would change the if statement here a bit like at other places in the build
so it is more generic. I will do that when Java 20 support comes, don't hurry!
Thanks Robert for fixing the alternate runtimes.
--
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
[GitHub] [lucene] dweiss commented on a diff in pull request #11937: reland: 11925 javac options
dweiss commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023671778
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
https://github.com/gradle/gradle/issues/22746
This is a bug in gradle - they treat jvm args differently, depending on the
compiler mode.
--
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
[GitHub] [lucene] dweiss commented on a diff in pull request #11937: reland: 11925 javac options
dweiss commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023673686
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
> They may not work on 9.x where altJvm + error-prone is actually possible
(in 10.x its not, hence impossible to test or worry about).
@rmuir altJvm is possible on 10.x as well, just pass a path to JDK17 via
-Pruntime.java.home=/my/jdk/17 and it'll use that JVM.
--
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
[GitHub] [lucene] jpountz commented on issue #11933: Remove IOContext from `Directory#openChecksumInput`?
jpountz commented on issue #11933: URL: https://github.com/apache/lucene/issues/11933#issuecomment-1316595426 @uschindler @rmuir I wonder if you have thoughts on this one. -- 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
[GitHub] [lucene] jpountz opened a new pull request, #11940: Address some error-prone NarrowingCompoundAssignment warnings.
jpountz opened a new pull request, #11940: URL: https://github.com/apache/lucene/pull/11940 Closes #11932 -- 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
[GitHub] [lucene] dweiss commented on a diff in pull request #11937: reland: 11925 javac options
dweiss commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r102369
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
> I would change the if statement here a bit like at other places in the
build so it is more generic. I will do that when Java 20 support comes, don't
hurry!
I pushed an alternative workaround that doesn't require any explicit task
references and is actually more direct as to what the hell actually is
happening there (with respect to the bug I filed with gradle).
--
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
[GitHub] [lucene] jpountz commented on a diff in pull request #11940: Address some error-prone NarrowingCompoundAssignment warnings.
jpountz commented on code in PR #11940:
URL: https://github.com/apache/lucene/pull/11940#discussion_r1023687106
##
lucene/core/src/java/org/apache/lucene/util/VectorUtil.java:
##
@@ -225,7 +225,7 @@ public static float[] l2normalize(float[] v, boolean
throwOnZero) {
return v;
}
}
-double length = Math.sqrt(squareSum);
+float length = (float) Math.sqrt(squareSum);
Review Comment:
I liked making this cast explicit to avoid giving excuses to the compiler
for not auto-vectorizing the below loop.
##
lucene/core/src/java/org/apache/lucene/search/PointRangeQuery.java:
##
@@ -415,7 +415,7 @@ private long pointCount(
BiFunction nodeComparator,
Predicate leafComparator)
throws IOException {
-final int[] matchingNodeCount = {0};
+final long[] matchingNodeCount = {0};
Review Comment:
This was not a bug yet since this method is only used for single-valued
fields currently, but could have become a bug if we started also using it for
multi-valued fields.
##
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##
@@ -136,12 +135,10 @@ private MergedVectorValues(List subs,
MergeState mergeState)
throws IOException {
this.subs = subs;
docIdMerger = DocIDMerger.of(subs, mergeState.needsIndexSort);
- int totalCost = 0, totalSize = 0;
Review Comment:
`cost` and `size` are actually the same to KNN vectors
##
lucene/core/src/java/org/apache/lucene/util/PagedBytes.java:
##
@@ -184,8 +184,10 @@ public void copy(IndexInput in, long byteCount) throws
IOException {
upto = blockSize;
byteCount -= left;
} else {
-in.readBytes(currentBlock, upto, (int) byteCount, false);
-upto += byteCount;
+// this cast is safe since byteCount is <= left at this point
+final int remainingByteCount = (int) byteCount;
+in.readBytes(currentBlock, upto, remainingByteCount, false);
+upto += remainingByteCount;
Review Comment:
I hesitated touching this one, I mostly did so in order to only perform the
cast once.
##
lucene/spatial-extras/src/java/org/apache/lucene/spatial/prefix/tree/PackedQuadPrefixTree.java:
##
@@ -281,7 +281,7 @@ protected void readLeafAdjust() {
public BytesRef getTokenBytesWithLeaf(BytesRef result) {
result = getTokenBytesNoLeaf(result);
if (isLeaf()) {
-result.bytes[8 - 1] |= 0x1L; // set leaf
+result.bytes[8 - 1] |= 0x1; // set leaf
Review Comment:
This is still a narrow cast, but from int to byte, rather than from long to
byte. It looked simpler to me to remove the `L`: why would you ensure it gets
treated as a long?
##
lucene/queries/src/java/org/apache/lucene/queries/spans/SpanScorer.java:
##
@@ -103,7 +103,7 @@ protected final void setFreqCurrentDoc() throws IOException
{
freq = 1;
return;
}
- freq += (1.0 / (1.0 + spans.width()));
+ freq += (1f / (1f + spans.width()));
Review Comment:
This diff could change scores a bit, but if we cared about the double
accuracy, we'd also sum up freqs in double space so I thought it improved
consistency to do everything in float space.
--
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
[GitHub] [lucene] jpountz commented on pull request #11940: Address some error-prone NarrowingCompoundAssignment warnings.
jpountz commented on PR #11940: URL: https://github.com/apache/lucene/pull/11940#issuecomment-1316631812 Oh, I'm just seeing #11938 now. Closing. -- 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
[GitHub] [lucene] jpountz closed pull request #11940: Address some error-prone NarrowingCompoundAssignment warnings.
jpountz closed pull request #11940: Address some error-prone NarrowingCompoundAssignment warnings. URL: https://github.com/apache/lucene/pull/11940 -- 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
[GitHub] [lucene] jpountz commented on a diff in pull request #11938: fix overflows in compound assignments
jpountz commented on code in PR #11938:
URL: https://github.com/apache/lucene/pull/11938#discussion_r1023705104
##
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##
@@ -108,7 +108,7 @@ public int nextDoc() throws IOException {
protected static class MergedVectorValues extends VectorValues {
private final List subs;
private final DocIDMerger docIdMerger;
-private final int cost;
+private final long cost;
Review Comment:
Maybe we should remove `cost` altogether here. It's redundant with `size` on
vectors?
--
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
[GitHub] [lucene] jpountz commented on pull request #11938: fix overflows in compound assignments
jpountz commented on PR #11938: URL: https://github.com/apache/lucene/pull/11938#issuecomment-1316638147 I fixed one other warning from PointRangeQuery which looked worth addressing. -- 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
[GitHub] [lucene] jpountz commented on pull request #11939: fix bug of incorrect cost after upgradeToBitSet in DocIdSetBuilder class
jpountz commented on PR #11939: URL: https://github.com/apache/lucene/pull/11939#issuecomment-1316641902 Great catch, can you add a test? -- 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
[GitHub] [lucene] maosuhan commented on pull request #11939: fix bug of incorrect cost after upgradeToBitSet in DocIdSetBuilder class
maosuhan commented on PR #11939: URL: https://github.com/apache/lucene/pull/11939#issuecomment-1316675899 > Great catch, can you add a test? @jpountz I have added the test 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
[GitHub] [lucene] jpountz commented on a diff in pull request #11928: GH#11922: Allow DisjunctionDISIApproximation to short-circuit
jpountz commented on code in PR #11928:
URL: https://github.com/apache/lucene/pull/11928#discussion_r1023716348
##
lucene/core/src/java/org/apache/lucene/search/DisjunctionDISIApproximation.java:
##
@@ -45,29 +51,54 @@ public long cost() {
@Override
public int docID() {
-return subIterators.top().doc;
+return docID;
}
- @Override
- public int nextDoc() throws IOException {
+ private int doNext(int target) throws IOException {
+if (target == DocIdSetIterator.NO_MORE_DOCS) {
+ docID = DocIdSetIterator.NO_MORE_DOCS;
+ return docID;
+}
+
DisiWrapper top = subIterators.top();
-final int doc = top.doc;
do {
- top.doc = top.approximation.nextDoc();
+ top.doc = top.approximation.advance(target);
+ if (top.doc == target) {
+subIterators.updateTop();
+docID = target;
+return docID;
+ }
top = subIterators.updateTop();
-} while (top.doc == doc);
+} while (top.doc < target);
+docID = top.doc;
-return top.doc;
+return docID;
+ }
+
+ @Override
+ public int nextDoc() throws IOException {
+if (docID == DocIdSetIterator.NO_MORE_DOCS) {
+ return docID;
+}
Review Comment:
This is not necessary, it's illegal to call `nextDoc` on an exhausted
`DocIdSetIterator`.
##
lucene/core/src/java/org/apache/lucene/search/DisjunctionDISIApproximation.java:
##
@@ -45,29 +51,54 @@ public long cost() {
@Override
public int docID() {
-return subIterators.top().doc;
+return docID;
}
- @Override
- public int nextDoc() throws IOException {
+ private int doNext(int target) throws IOException {
+if (target == DocIdSetIterator.NO_MORE_DOCS) {
+ docID = DocIdSetIterator.NO_MORE_DOCS;
+ return docID;
+}
Review Comment:
In general I'm dubious about such checks for `NO_MORE_DOCS` since we need to
do this check on every candidate match but this condition is true at most once
per segment, and often never. So it looks like extra overhead that doesn't
bring significant value, and could prevent e.g. inlining due to increased
bytecode 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
[GitHub] [lucene] romseygeek commented on issue #11864: ArrayIndexOutOfBoundException
romseygeek commented on issue #11864: URL: https://github.com/apache/lucene/issues/11864#issuecomment-1316715728 Ah, I see what you mean. I think we can improve the internal API here to not rely on there being a non-empty TermAndBoost array when constructing the synonym query. I'll open a pull request. #9763 is a bigger problem, unfortunately. -- 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
[GitHub] [lucene] uschindler commented on issue #11933: Remove IOContext from `Directory#openChecksumInput`?
uschindler commented on issue #11933: URL: https://github.com/apache/lucene/issues/11933#issuecomment-1316719591 Hi, I think the idea of IOContext was beyond simple flags like "sequentially reading" or "random IO" (and you can create your own IOContexts). I think we added that to allow full flexibility. For the checkum case it may be predefined, so yes, we could think of removing the argument. I am +/-0 for removing it, on the other hand it does not hurt to have it for future extensibility. Do you have something in mind where it may hinder 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
[GitHub] [lucene] jpountz commented on issue #11933: Remove IOContext from `Directory#openChecksumInput`?
jpountz commented on issue #11933: URL: https://github.com/apache/lucene/issues/11933#issuecomment-1316724527 I opened this issue because I found several instances of codecs opening their meta file with the default `IOContext` instead of using `READONCE`. https://github.com/apache/lucene/pull/11934 If we did not have the `IOContext` argument on `openChecksumInput`, these bugs would not exist. -- 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
[GitHub] [lucene] thecoop commented on pull request #11847: Add a method allowing canonical strings to be returned from DataInput
thecoop commented on PR #11847: URL: https://github.com/apache/lucene/pull/11847#issuecomment-1316834475 I've changed it to use a separate object for the string map, scoped to the deserialisation method. I couldn't come up with a nice way to handle `readMapOfStrings` to cache the key strings so far, it's much harder to do that once the map is actually created. -- 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
[GitHub] [lucene] uschindler commented on a diff in pull request #11937: reland: 11925 javac options
uschindler commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023893339
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
It is a5e25259468 ? Cool. Much better as it works with toolchains, too.
Can you backport this 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
[GitHub] [lucene] uschindler commented on issue #11933: Remove IOContext from `Directory#openChecksumInput`?
uschindler commented on issue #11933: URL: https://github.com/apache/lucene/issues/11933#issuecomment-1316889438 I think we can remove it, but if we keep it maybe it should always use READONCE. -- 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
[GitHub] [lucene] rmuir commented on a diff in pull request #11937: reland: 11925 javac options
rmuir commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023903448
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
> @rmuir altJvm is possible on 10.x as well, just pass a path to JDK17 via
-Pruntime.java.home=/my/jdk/17 and it'll use that JVM.
yes, i know, but i was trying to test the grid of `altjvm=True/False X
errorprone=True/False` on 10.x, you can't do altjvm=True + errorprone=True, it
gets disabled due to some workaround. So I tested that on 9.x
thanks for looking at it, as long as jenkins is happy we are good.
--
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
[GitHub] [lucene] rmuir commented on a diff in pull request #11937: reland: 11925 javac options
rmuir commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023906901
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
since you defer the evaluation, i think your change will work fine!
--
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
[GitHub] [lucene] rmuir commented on a diff in pull request #11938: fix overflows in compound assignments
rmuir commented on code in PR #11938:
URL: https://github.com/apache/lucene/pull/11938#discussion_r1023927808
##
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##
@@ -108,7 +108,7 @@ public int nextDoc() throws IOException {
protected static class MergedVectorValues extends VectorValues {
private final List subs;
private final DocIDMerger docIdMerger;
-private final int cost;
+private final long cost;
Review Comment:
i didn't look at how it was used, only doing shallow cleanups here. it is
exposed via `long cost()` method, so it never made sense to be an int. what
would `cost()` return instead?
--
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
[GitHub] [lucene] thecoop opened a new pull request, #11942: Ensure collections are properly sized on creation
thecoop opened a new pull request, #11942: URL: https://github.com/apache/lucene/pull/11942 Change calls to `new HashMap(int)`/`new HashSet(int)` to a method that ensures the backing array won't be resized. A few other optimisations around map methods I picked up along the way -- 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
[GitHub] [lucene] uschindler commented on a diff in pull request #11937: reland: 11925 javac options
uschindler commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023954625
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
> > @rmuir altJvm is possible on 10.x as well, just pass a path to JDK17 via
-Pruntime.java.home=/my/jdk/17 and it'll use that JVM.
>
> yes, i know, but i was trying to test the grid of `altjvm=True/False X
errorprone=True/False` on 10.x, you can't do altjvm=True + errorprone=True, it
gets disabled due to some workaround. So I tested that on 9.x
>
> thanks for looking at it, as long as jenkins is happy we are good.
Yes altjvm does not work with errorprone due to a bug in the errorprone
plugin. the issue has to do with the open modules. Maybe it is the same issue
like with -J and no -J.
We disabled it. For Gradle's Toolkit, theres support in the errorprone
plugin. It just does not work with our altjvm support.
--
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
[GitHub] [lucene] rmuir commented on a diff in pull request #11937: reland: 11925 javac options
rmuir commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023957903
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
yeah, i just wanted the change to not be a PITA in the future in case the
bug got fixed and workaround removed. but i was surprised by the compileMain19
and had to hack around that in an ugly way. Dawid's patch does what i
originally wanted to do, just deferred so it doesn't get overwritten.
--
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
[GitHub] [lucene] dweiss commented on a diff in pull request #11937: reland: 11925 javac options
dweiss commented on code in PR #11937:
URL: https://github.com/apache/lucene/pull/11937#discussion_r1023961947
##
gradle/hacks/turbocharge-jvm-opts.gradle:
##
@@ -38,4 +39,20 @@ allprojects {
jvmArgs += vmOpts
}
-}
\ No newline at end of file
+
+// Tweak javac to not be too resource-hungry.
+// This applies to any JVM when javac runs forked (e.g. error-prone)
+// Avoiding the fork entirely is best.
+tasks.withType(JavaCompile) { JavaCompile task ->
+if (task.path == ":lucene:core:compileMain19Java") {
Review Comment:
I'll backport, no problem.
--
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
[GitHub] [lucene] jpountz commented on a diff in pull request #11938: fix overflows in compound assignments
jpountz commented on code in PR #11938:
URL: https://github.com/apache/lucene/pull/11938#discussion_r1023992252
##
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##
@@ -108,7 +108,7 @@ public int nextDoc() throws IOException {
protected static class MergedVectorValues extends VectorValues {
private final List subs;
private final DocIDMerger docIdMerger;
-private final int cost;
+private final long cost;
Review Comment:
I pushed a commit with what I had in mind. Essentially `cost()` and `size()`
are the same thing on vectors except that the latter is expected to be
accurate. Maybe we should rethink this, e.g. could `cost()` be required to be
accurate on `VectorValues` instances, or could the base class make `cost()`
final and delegate to `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
[GitHub] [lucene] jpountz commented on pull request #11939: fix bug of incorrect cost after upgradeToBitSet in DocIdSetBuilder class
jpountz commented on PR #11939: URL: https://github.com/apache/lucene/pull/11939#issuecomment-1317027842 This looks good to me. Can you add a CHANGES entry under `9.4.2`? -- 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
[GitHub] [lucene] mikemccand commented on issue #11933: Remove IOContext from `Directory#openChecksumInput`?
mikemccand commented on issue #11933: URL: https://github.com/apache/lucene/issues/11933#issuecomment-1317075989 +1 to remove 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
[GitHub] [lucene] rmuir commented on a diff in pull request #11938: fix overflows in compound assignments
rmuir commented on code in PR #11938:
URL: https://github.com/apache/lucene/pull/11938#discussion_r1024068862
##
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##
@@ -108,7 +108,7 @@ public int nextDoc() throws IOException {
protected static class MergedVectorValues extends VectorValues {
private final List subs;
private final DocIDMerger docIdMerger;
-private final int cost;
+private final long cost;
Review Comment:
i'm happy either way, i'm not familiar enough with the specifics of the
code, and I lean conservative when trying to fix issues before a release. we
don't even have to touch it here now if there's no real risk of overflow (e.g.
its just count of subs).
--
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
[GitHub] [lucene] maosuhan commented on pull request #11939: fix bug of incorrect cost after upgradeToBitSet in DocIdSetBuilder class
maosuhan commented on PR #11939: URL: https://github.com/apache/lucene/pull/11939#issuecomment-1317154566 @jpountz changed -- 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
[GitHub] [lucene] rmuir commented on pull request #11938: fix overflows in compound assignments
rmuir commented on PR #11938: URL: https://github.com/apache/lucene/pull/11938#issuecomment-1317237989 Thanks @jpountz for going thru the file with 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
[GitHub] [lucene] rmuir closed issue #11932: one-time pass thru error-prone NarrowingCompoundAssignment check
rmuir closed issue #11932: one-time pass thru error-prone NarrowingCompoundAssignment check URL: https://github.com/apache/lucene/issues/11932 -- 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
[GitHub] [lucene] rmuir merged pull request #11938: fix overflows in compound assignments
rmuir merged PR #11938: URL: https://github.com/apache/lucene/pull/11938 -- 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
[GitHub] [lucene] jpountz merged pull request #11939: fix bug of incorrect cost after upgradeToBitSet in DocIdSetBuilder class
jpountz merged PR #11939: URL: https://github.com/apache/lucene/pull/11939 -- 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
[GitHub] [lucene] rmuir opened a new issue, #11943: try to re-enable error-prone checks to happen more often?
rmuir opened a new issue, #11943: URL: https://github.com/apache/lucene/issues/11943 ### Description Currently this is enabled nightly-only, and it is very slow compared to java compiler. I wonder if we could enable it "more often" somehow. Besides being slow it also has some compatibility issues with different JDK versions I think, or at least it has in the past. So this could be a problem to an actual end-user of lucene... We sped it up with #11937 and it no longer dominates the whole machine in such a way that you can't move the mouse pointer when it runs. Maybe it could be sped up more if we disabled additional checks. I feel like the way we denylist checks, new checks sneak in that we may not want on upgrading it, making it slower. Maybe a compromise would be to enable it just for github actions? -- 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
[GitHub] [lucene] dweiss commented on issue #11943: try to re-enable error-prone checks to happen more often?
dweiss commented on issue #11943: URL: https://github.com/apache/lucene/issues/11943#issuecomment-1317401162 > Maybe a compromise would be to enable it just for github actions? I'm fine with this. Doesn't bother me to push and do something else while waiting for CI to complete its checks. -- 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
[GitHub] [lucene] rmuir commented on issue #11943: try to re-enable error-prone checks to happen more often?
rmuir commented on issue #11943: URL: https://github.com/apache/lucene/issues/11943#issuecomment-1317413881 Looks like we tend to spin off two "big" jobs in parallel: `gradle test` and `gradle check -x test`. The `gradle test` job tends to run a bit slower already, so it may not hurt overall "throughput" much to enable it on the `gradle check -x test` precommit job. I'll put out a 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
[GitHub] [lucene] rmuir opened a new pull request, #11944: enable errorprone for github check action
rmuir opened a new pull request, #11944: URL: https://github.com/apache/lucene/pull/11944 The idea is to fail faster, prevent new violations from failing nightly builds. If it becomes a hindrance to contributions or an annoyance, we can disable it. Closes #11943 -- 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
[GitHub] [lucene] risdenk commented on pull request #11944: enable errorprone for github check action
risdenk commented on PR #11944: URL: https://github.com/apache/lucene/pull/11944#issuecomment-1317463797 Looking at the last main github actions run: https://github.com/apache/lucene/actions/runs/3481009561/jobs/5821544088 The gradle check without tests was `6m 32s`. The same step for this github actions run: https://github.com/apache/lucene/actions/runs/3481792417/jobs/5823331065 was `6m 37s` So 5s added from what I can tell now. Based on the compile times doesn't look like the 5 seconds was from compile either. -- 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
[GitHub] [lucene] dweiss commented on pull request #11944: enable errorprone for github check action
dweiss commented on PR #11944: URL: https://github.com/apache/lucene/pull/11944#issuecomment-1317507423 Well, it was definitely slower before the patch ()? https://github.com/apache/lucene/actions/runs/3473485571/jobs/5805548845 -- 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
[GitHub] [lucene] rmuir commented on pull request #11944: enable errorprone for github check action
rmuir commented on PR #11944: URL: https://github.com/apache/lucene/pull/11944#issuecomment-1317507534 hmm... so did it actually run? :) fwiw, my biggest concern for this is annoying contributors. if they do the right thing and `gradle check` fails, then make a contribution and the build fails here, it could bother them. Especially if they have to play whack-a-mole because error-prone feeds them one violation at a time :( At the same time, we don't have a lot of crazy error-prone checks turned on... they are selective... so hopefully it would be more of a rare event. -- 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
[GitHub] [lucene] dweiss commented on pull request #11944: enable errorprone for github check action
dweiss commented on PR #11944: URL: https://github.com/apache/lucene/pull/11944#issuecomment-1317510122 > hmm... so did it actually run? :) You could try to revert the commit with offending code - then we'd see. :) -- 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
[GitHub] [lucene] rmuir commented on pull request #11944: enable errorprone for github check action
rmuir commented on PR #11944: URL: https://github.com/apache/lucene/pull/11944#issuecomment-1317515764 i will add a violation -- 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
[GitHub] [lucene] jpountz commented on pull request #912: MR-JAR rewrite of MMapDirectory with JDK-19 preview Panama APIs (>= JDK-19-ea+23)
jpountz commented on PR #912: URL: https://github.com/apache/lucene/pull/912#issuecomment-1317520371 @uschindler It looks like this benchmark has a noticeable slowdown with this change? https://home.apache.org/~mikemccand/lucenebench/MedTermDayTaxoFacets.html -- 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
[GitHub] [lucene] rmuir commented on pull request #11944: enable errorprone for github check action
rmuir commented on PR #11944: URL: https://github.com/apache/lucene/pull/11944#issuecomment-1317529801 If it works, build should fail and look like this: ``` > Task :lucene:core:compileTestJava FAILED /home/rmuir/workspace/lucene/lucene/core/src/test/org/apache/lucene/TestDemo.java:76: warning: [ConstantOverflow] Compile-time constant expression overflows long x = Integer.MAX_VALUE * Integer.MAX_VALUE; ^ (see https://errorprone.info/bugpattern/ConstantOverflow) error: warnings found and -Werror specified Note: Some input files use or override a deprecated API. Note: Recompile with -Xlint:deprecation for details. 1 error 1 warning FAILURE: Build failed with an exception. ``` -- 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
[GitHub] [lucene] rmuir commented on pull request #11944: enable errorprone for github check action
rmuir commented on PR #11944: URL: https://github.com/apache/lucene/pull/11944#issuecomment-1317535353 It worked... even for the `gradle test` task which didn't turn it on. So now i'm wondering if it has been enabled here all along because of `isCIBuild` which has a "CI-detector" that looks at env vars. -- 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
[GitHub] [lucene] rmuir commented on pull request #11944: enable errorprone for github check action
rmuir commented on PR #11944:
URL: https://github.com/apache/lucene/pull/11944#issuecomment-1317542732
yeah, nothing to do here apparently. errorprone is already on via magical CI
detection: `isCIBuild = System.getenv().keySet().find { it ==~
/(?i)((JENKINS|HUDSON)(_\w+)?|CI)/ } != null`
--
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
[GitHub] [lucene] rmuir closed pull request #11944: enable errorprone for github check action
rmuir closed pull request #11944: enable errorprone for github check action URL: https://github.com/apache/lucene/pull/11944 -- 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
[GitHub] [lucene] rmuir closed issue #11943: try to re-enable error-prone checks to happen more often?
rmuir closed issue #11943: try to re-enable error-prone checks to happen more often? URL: https://github.com/apache/lucene/issues/11943 -- 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
[GitHub] [lucene] rmuir commented on issue #11943: try to re-enable error-prone checks to happen more often?
rmuir commented on issue #11943: URL: https://github.com/apache/lucene/issues/11943#issuecomment-1317543404 error-prone is already enabled via ci-detection magic :) -- 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
[GitHub] [lucene] dweiss commented on issue #11943: try to re-enable error-prone checks to happen more often?
dweiss commented on issue #11943: URL: https://github.com/apache/lucene/issues/11943#issuecomment-1317546712 Ahem. Right. Now, where's that facepalm emoji... -- 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
[GitHub] [lucene] uschindler commented on pull request #912: MR-JAR rewrite of MMapDirectory with JDK-19 preview Panama APIs (>= JDK-19-ea+23)
uschindler commented on PR #912: URL: https://github.com/apache/lucene/pull/912#issuecomment-1317618039 Could be. It may be caused by too much coping of very small arrays (especially float and long) between offheap and heap. For explanation see above. I don't think this is a huge problem because it mainly comes from missing optimizations in the JVM and not our code. Could look different in Java 20. My only guess where it might come from: The overhead of copyMemory is huge for small arrays. ByteBuffer internally loops for very short arrays. We could try to call super.readLongs() if array is short (same for floats). But before doing this I would like to figure out what exactly slows. If I could get a profile only from this test or somebody could explain what mmap dir actions are used mainly in those facets. -- 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
[GitHub] [lucene] uschindler commented on pull request #912: MR-JAR rewrite of MMapDirectory with JDK-19 preview Panama APIs (>= JDK-19-ea+23)
uschindler commented on PR #912: URL: https://github.com/apache/lucene/pull/912#issuecomment-1317634632 Code similar like this: https://github.com/openjdk/jdk/blob/37848a9ca2ab3021e7b3b2e112bab4631fbe1d99/src/java.base/share/classes/java/nio/X-Buffer.java.template#L929 The of statement uses a simple copy loop to read the values if length is too short. We can do the same in our code by calling super to read into array. It would be worth a try. -- 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
[GitHub] [lucene] uschindler commented on pull request #912: MR-JAR rewrite of MMapDirectory with JDK-19 preview Panama APIs (>= JDK-19-ea+23)
uschindler commented on PR #912: URL: https://github.com/apache/lucene/pull/912#issuecomment-1317635619 I was hoping that JDK people do it in the same way for MemorySegment. -- 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
[GitHub] [lucene] gsmiller commented on pull request #11901: Github#11869: Add RangeOnRangeFacetCounts
gsmiller commented on PR #11901: URL: https://github.com/apache/lucene/pull/11901#issuecomment-1317848376 @mdmarshmallow started to dig into this. Will get you some feedback this week. Just letting you know it's on my radar. Thanks for all the hard work on this! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@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
[GitHub] [lucene] jdconrad opened a new pull request, #11945: Decrease test time for TestManyKnnDocs.testLargeSegment
jdconrad opened a new pull request, #11945: URL: https://github.com/apache/lucene/pull/11945 This change adds an additional test codec allowing a configurable number for max connections per vector when building an hnsw index. By setting the number of connections to `128` as part of `TestManyKnnDocs.testLargeSegment` we can reduce the number of indexed vectors to `2088992` and still reproduce the test failure prior to the fix by @benwtrent in https://github.com/apache/lucene/pull/11905. This changed reduced the test time for me from ~90 minutes to ~3 minutes locally. cc @rmuir -- 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
[GitHub] [lucene] rmuir commented on pull request #11945: Decrease test time for TestManyKnnDocs.testLargeSegment
rmuir commented on PR #11945: URL: https://github.com/apache/lucene/pull/11945#issuecomment-1317994569 we need to run a `./gradlew tidy` and commit/push the results to fix formatting. Very cool, will test on my 2-core. we may be able to upgrade from `@Monster` to `@Nightly`. -- 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
[GitHub] [lucene] jdconrad commented on pull request #11945: Decrease test time for TestManyKnnDocs.testLargeSegment
jdconrad commented on PR #11945: URL: https://github.com/apache/lucene/pull/11945#issuecomment-1318001086 Updated with tidy! (Oops on failing precommit.) -- 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
[GitHub] [lucene] rmuir commented on pull request #11945: Decrease test time for TestManyKnnDocs.testLargeSegment
rmuir commented on PR #11945: URL: https://github.com/apache/lucene/pull/11945#issuecomment-1318068758 Works for me. I was able to now run this monster test in < 10 minutes time. ``` <===:lucene:core:test (SUCCESS): 1 test(s) The slowest tests (exceeding 500 ms) during this run: 527.29s TestManyKnnDocs.testLargeSegment (:lucene:core) The slowest suites (exceeding 1s) during this run: 527.61s TestManyKnnDocs (:lucene:core) BUILD SUCCESSFUL in 9m 2s 19 actionable tasks: 5 executed, 14 up-to-date ``` -- 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
[GitHub] [lucene] rmuir merged pull request #11945: Decrease test time for TestManyKnnDocs.testLargeSegment
rmuir merged PR #11945: URL: https://github.com/apache/lucene/pull/11945 -- 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
