Re: [VOTE] Release Lucene 9.6.0 RC1
+1. Thanks Alan! SUCCESS! [0:48:46.465752] On Tue, May 2, 2023 at 11:33 AM Alan Woodward wrote: > Please vote for release candidate 1 for Lucene 9.6.0 > > The artifacts can be downloaded from: > > https://dist.apache.org/repos/dist/dev/lucene/lucene-9.6.0-RC1-rev-246ac4bcbe6c0ea7bebcc4f90b2be0536f00d842 > > You can run the smoke tester directly with this command: > > python3 -u dev-tools/scripts/smokeTestRelease.py \ > > https://dist.apache.org/repos/dist/dev/lucene/lucene-9.6.0-RC1-rev-246ac4bcbe6c0ea7bebcc4f90b2be0536f00d842 > > It is another long holiday weekend here in the UK, so the vote is likely > to be held open until at least Monday. > > [ ] +1 approve > [ ] +0 no opinion > [ ] -1 disapprove (and reason why) > > Here is my +1 > > > - > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > >
Re: Unnecessary float[256] allocation on every (non-scoring) BM25Scorer
> So I'm actually still confused why this float[256] stands out in your > measurejments vs two long[128]'s. Maybe its a profiler ghost? Huh... that's a really good point. I'm going to spend a bit more time digging and see if I can reliably reproduce it on my own machine. I've just been comparing heap dumps from production hosts so far, so I'll try measuring in an environment where I can see what's going on. On Tue, May 2, 2023 at 1:14 PM Robert Muir wrote: > On Tue, May 2, 2023 at 3:24 PM Michael Froh wrote: > > > > > This seems ok if it isn't invasive. I still feel like something is > > > "off" if you are seeing GC time from 1KB-per-segment allocation. Do > > > you have way too many segments? > > > > From what I saw, it's 1KB per "leaf query" to create the BM25Scorer > instance (at the Weight level), but then that BM25Scorer is shared across > all scorer (DISI) instances for all segments. So it doesn't scale with > segment count. It looks like the old logic used to allocate a SimScorer per > segment, so this is a big improvement in that regard (for scoring clauses, > since the non-scoring clauses had a super-lightweight SimScorer). > > > > In this particular case, they're running these gnarly machine-generated > BoolenQuery trees with at least 512 non-scoring TermQuery clauses (across a > bunch of different fields, so TermInSetQuery isn't an option). From what I > can see, each of those TermQueries produces a TermWeight that holds a > BM25Scorer that holds yet another instance of this float[256] array, for > 512KB+ of these caches per running query. It's definitely only going to be > an issue for folks who are flying close to the max clause count. > > > > Yeah, but the same situation could be said for buffers like this one: > > https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90PostingsReader.java#L311-L312 > So I'm actually still confused why this float[256] stands out in your > measurejments vs two long[128]'s. Maybe its a profiler ghost? > > - > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > >
Re: Why two TermAndBoost classes
Adding to cc Daniele who worked on this quite recently! Cheers On Tue, 2 May 2023, 21:12 Gus Heck, wrote: > Was fishing around in parsers in solr and discovered that we have two > different term and boost classes in Lucene. Is this really desirable? They > are quite similar except one implements a notion of equality, and doesn't > copy the BytesRef when created whereas the other relies on object equality > and does copy the BytesRef in the constructor. > > The difference in copying BytesRef seems suspicious, and I wonder if > there's a good reason not to have a different notion of equality among the > two. Also one is public and the other is private to SynonymQuery but both > are static and don't seem to leverage their privileges of being within the > containing class, so maybe they don't need to be inner classes? > > -Gus > > -- > http://www.needhamsoftware.com (work) > http://www.the111shift.com (play) >
Re: Unnecessary float[256] allocation on every (non-scoring) BM25Scorer
On Tue, May 2, 2023 at 3:24 PM Michael Froh wrote: > > > This seems ok if it isn't invasive. I still feel like something is > > "off" if you are seeing GC time from 1KB-per-segment allocation. Do > > you have way too many segments? > > From what I saw, it's 1KB per "leaf query" to create the BM25Scorer instance > (at the Weight level), but then that BM25Scorer is shared across all scorer > (DISI) instances for all segments. So it doesn't scale with segment count. It > looks like the old logic used to allocate a SimScorer per segment, so this is > a big improvement in that regard (for scoring clauses, since the non-scoring > clauses had a super-lightweight SimScorer). > > In this particular case, they're running these gnarly machine-generated > BoolenQuery trees with at least 512 non-scoring TermQuery clauses (across a > bunch of different fields, so TermInSetQuery isn't an option). From what I > can see, each of those TermQueries produces a TermWeight that holds a > BM25Scorer that holds yet another instance of this float[256] array, for > 512KB+ of these caches per running query. It's definitely only going to be an > issue for folks who are flying close to the max clause count. > Yeah, but the same situation could be said for buffers like this one: https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90PostingsReader.java#L311-L312 So I'm actually still confused why this float[256] stands out in your measurejments vs two long[128]'s. Maybe its a profiler ghost? - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Why two TermAndBoost classes
Was fishing around in parsers in solr and discovered that we have two different term and boost classes in Lucene. Is this really desirable? They are quite similar except one implements a notion of equality, and doesn't copy the BytesRef when created whereas the other relies on object equality and does copy the BytesRef in the constructor. The difference in copying BytesRef seems suspicious, and I wonder if there's a good reason not to have a different notion of equality among the two. Also one is public and the other is private to SynonymQuery but both are static and don't seem to leverage their privileges of being within the containing class, so maybe they don't need to be inner classes? -Gus -- http://www.needhamsoftware.com (work) http://www.the111shift.com (play)
Re: Unnecessary float[256] allocation on every (non-scoring) BM25Scorer
> This seems ok if it isn't invasive. I still feel like something is > "off" if you are seeing GC time from 1KB-per-segment allocation. Do > you have way too many segments? >From what I saw, it's 1KB per "leaf query" to create the BM25Scorer instance (at the Weight level), but then that BM25Scorer is shared across all scorer (DISI) instances for all segments. So it doesn't scale with segment count. It looks like the old logic used to allocate a SimScorer per segment, so this is a big improvement in that regard (for scoring clauses, since the non-scoring clauses had a super-lightweight SimScorer). In this particular case, they're running these gnarly machine-generated BoolenQuery trees with at least 512 non-scoring TermQuery clauses (across a bunch of different fields, so TermInSetQuery isn't an option). From what I can see, each of those TermQueries produces a TermWeight that holds a BM25Scorer that holds yet another instance of this float[256] array, for 512KB+ of these caches per running query. It's definitely only going to be an issue for folks who are flying close to the max clause count. > One last thought: we should re-check if the cache is still needed :) I > think decoding norms used to be more expensive in the past. This cache > is now only precomputing part of the bm25 formula to save some > add/multiply/divide. Yeah -- when I saw the cache calculation, it reminded me of precomputed tables of trigonometric functions in the demoscene. I could try inlining those calculations and measuring the impact with the luceneutil benchmarks. On Tue, May 2, 2023 at 11:34 AM Robert Muir wrote: > On Tue, May 2, 2023 at 12:49 PM Michael Froh wrote: > > > > Hi all, > > > > I was looking into a customer issue where they noticed some increased GC > time after upgrading from Lucene 7.x to 9.x. After taking some heap dumps > from both systems, the big difference was tracked down to the float[256] > allocated (as a norms cache) when creating a BM25Scorer (in > BM25Similarity.scorer()). > > > > The change seems to have come in with > https://github.com/apache/lucene/commit/8fd7ead940f69a892dfc951a1aa042e8cae806c1, > which removed some of the special-case logic around the "non-scoring > similarity" embedded in IndexSearcher (returned in the false case from the > old IndexSearcher#scorer(boolean needsScores)). > > > > While I really like that we no longer have that special-case logic in > IndexSearcher, we now have the issue that every time we create a new > TermWeight (or other Weight) it allocates a float[256], even if the > TermWeight doesn't need scores. Also, I think it's the exact same > float[256] for all non-scoring weights, since it's being computed using the > same "all 1s" CollectionStatistics and TermStatistics. > > > > (For the record, yes, the queries in question have an obscene number of > TermQueries, so 1024 bytes times lots of TermWeights, times multiple > queries running concurrently makes lots of heap allocation.) > > > > I'd like to submit a patch to fix this, but I'm wondering what approach > to take. One option I'm considering is precomputing a singleton float[256] > for the non-scoring case (where CollectionStatistics and TermStatistics are > all 1s). That would have the least functional impact, but would let all > non-scoring clauses share the same array. Is there a better way to tackle > this? > > > > This seems ok if it isn't invasive. I still feel like something is > "off" if you are seeing GC time from 1KB-per-segment allocation. Do > you have way too many segments? > > Originally (for various similar reasons) there was a place in the API > to do this, so it would only happen per-Weight instead of per-Scorer, > which was the SimWeight that got eliminated by the commit you point > to. But I'd love if we could steer clear of that complexity: > simplifying the API here was definitely the right move. Its been more > than 5 years since this change was made, and this is the first > complaint i've heard about the 1KB, which is why i asked about your > setup. > > - > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > >
Re: Unnecessary float[256] allocation on every (non-scoring) BM25Scorer
On Tue, May 2, 2023 at 2:34 PM Robert Muir wrote: > > On Tue, May 2, 2023 at 12:49 PM Michael Froh wrote: > > > > Hi all, > > > > I was looking into a customer issue where they noticed some increased GC > > time after upgrading from Lucene 7.x to 9.x. After taking some heap dumps > > from both systems, the big difference was tracked down to the float[256] > > allocated (as a norms cache) when creating a BM25Scorer (in > > BM25Similarity.scorer()). > > > > The change seems to have come in with > > https://github.com/apache/lucene/commit/8fd7ead940f69a892dfc951a1aa042e8cae806c1, > > which removed some of the special-case logic around the "non-scoring > > similarity" embedded in IndexSearcher (returned in the false case from the > > old IndexSearcher#scorer(boolean needsScores)). > > > > While I really like that we no longer have that special-case logic in > > IndexSearcher, we now have the issue that every time we create a new > > TermWeight (or other Weight) it allocates a float[256], even if the > > TermWeight doesn't need scores. Also, I think it's the exact same > > float[256] for all non-scoring weights, since it's being computed using the > > same "all 1s" CollectionStatistics and TermStatistics. > > > > (For the record, yes, the queries in question have an obscene number of > > TermQueries, so 1024 bytes times lots of TermWeights, times multiple > > queries running concurrently makes lots of heap allocation.) > > > > I'd like to submit a patch to fix this, but I'm wondering what approach to > > take. One option I'm considering is precomputing a singleton float[256] for > > the non-scoring case (where CollectionStatistics and TermStatistics are all > > 1s). That would have the least functional impact, but would let all > > non-scoring clauses share the same array. Is there a better way to tackle > > this? > > > > This seems ok if it isn't invasive. I still feel like something is > "off" if you are seeing GC time from 1KB-per-segment allocation. Do > you have way too many segments? > > Originally (for various similar reasons) there was a place in the API > to do this, so it would only happen per-Weight instead of per-Scorer, > which was the SimWeight that got eliminated by the commit you point > to. But I'd love if we could steer clear of that complexity: > simplifying the API here was definitely the right move. Its been more > than 5 years since this change was made, and this is the first > complaint i've heard about the 1KB, which is why i asked about your > setup. One last thought: we should re-check if the cache is still needed :) I think decoding norms used to be more expensive in the past. This cache is now only precomputing part of the bm25 formula to save some add/multiply/divide. - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Re: Unnecessary float[256] allocation on every (non-scoring) BM25Scorer
On Tue, May 2, 2023 at 12:49 PM Michael Froh wrote: > > Hi all, > > I was looking into a customer issue where they noticed some increased GC time > after upgrading from Lucene 7.x to 9.x. After taking some heap dumps from > both systems, the big difference was tracked down to the float[256] allocated > (as a norms cache) when creating a BM25Scorer (in BM25Similarity.scorer()). > > The change seems to have come in with > https://github.com/apache/lucene/commit/8fd7ead940f69a892dfc951a1aa042e8cae806c1, > which removed some of the special-case logic around the "non-scoring > similarity" embedded in IndexSearcher (returned in the false case from the > old IndexSearcher#scorer(boolean needsScores)). > > While I really like that we no longer have that special-case logic in > IndexSearcher, we now have the issue that every time we create a new > TermWeight (or other Weight) it allocates a float[256], even if the > TermWeight doesn't need scores. Also, I think it's the exact same float[256] > for all non-scoring weights, since it's being computed using the same "all > 1s" CollectionStatistics and TermStatistics. > > (For the record, yes, the queries in question have an obscene number of > TermQueries, so 1024 bytes times lots of TermWeights, times multiple queries > running concurrently makes lots of heap allocation.) > > I'd like to submit a patch to fix this, but I'm wondering what approach to > take. One option I'm considering is precomputing a singleton float[256] for > the non-scoring case (where CollectionStatistics and TermStatistics are all > 1s). That would have the least functional impact, but would let all > non-scoring clauses share the same array. Is there a better way to tackle > this? > This seems ok if it isn't invasive. I still feel like something is "off" if you are seeing GC time from 1KB-per-segment allocation. Do you have way too many segments? Originally (for various similar reasons) there was a place in the API to do this, so it would only happen per-Weight instead of per-Scorer, which was the SimWeight that got eliminated by the commit you point to. But I'd love if we could steer clear of that complexity: simplifying the API here was definitely the right move. Its been more than 5 years since this change was made, and this is the first complaint i've heard about the 1KB, which is why i asked about your setup. - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[VOTE] Release Lucene 9.6.0 RC1
Please vote for release candidate 1 for Lucene 9.6.0 The artifacts can be downloaded from: https://dist.apache.org/repos/dist/dev/lucene/lucene-9.6.0-RC1-rev-246ac4bcbe6c0ea7bebcc4f90b2be0536f00d842 You can run the smoke tester directly with this command: python3 -u dev-tools/scripts/smokeTestRelease.py \ https://dist.apache.org/repos/dist/dev/lucene/lucene-9.6.0-RC1-rev-246ac4bcbe6c0ea7bebcc4f90b2be0536f00d842 It is another long holiday weekend here in the UK, so the vote is likely to be held open until at least Monday. [ ] +1 approve [ ] +0 no opinion [ ] -1 disapprove (and reason why) Here is my +1 - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
Unnecessary float[256] allocation on every (non-scoring) BM25Scorer
Hi all, I was looking into a customer issue where they noticed some increased GC time after upgrading from Lucene 7.x to 9.x. After taking some heap dumps from both systems, the big difference was tracked down to the float[256] allocated (as a norms cache) when creating a BM25Scorer (in BM25Similarity.scorer()). The change seems to have come in with https://github.com/apache/lucene/commit/8fd7ead940f69a892dfc951a1aa042e8cae806c1, which removed some of the special-case logic around the "non-scoring similarity" embedded in IndexSearcher (returned in the false case from the old IndexSearcher#scorer(boolean needsScores)). While I really like that we no longer have that special-case logic in IndexSearcher, we now have the issue that every time we create a new TermWeight (or other Weight) it allocates a float[256], even if the TermWeight doesn't need scores. Also, I think it's the exact same float[256] for all non-scoring weights, since it's being computed using the same "all 1s" CollectionStatistics and TermStatistics. (For the record, yes, the queries in question have an obscene number of TermQueries, so 1024 bytes times lots of TermWeights, times multiple queries running concurrently makes lots of heap allocation.) I'd like to submit a patch to fix this, but I'm wondering what approach to take. One option I'm considering is precomputing a singleton float[256] for the non-scoring case (where CollectionStatistics and TermStatistics are all 1s). That would have the least functional impact, but would let all non-scoring clauses share the same array. Is there a better way to tackle this? Thanks, Froh
Re: New branch and feature freeze for Lucene 9.6.0
Don't remember the specifics, but I ran into GPG issues during Solr 9.1.0 release. The fix for me was https://github.com/apache/solr/pull/1125, but I don't know if this is the same problem or if it is applicable in Lucene's case. On Tue, 2 May 2023 at 18:27, Alan Woodward wrote: > I am fighting with gradle and GPG yet again… Gradle fails when trying to > sign artefacts with the message "Cannot perform signing task > ':lucene:distribution:signReleaseArchives' because it has no configured > signatory”. I have GPG configured in ~/.gradle/gradle.properties as > follows: > > org.gradle.caching=true > signing.keyId= > signing.secretKeyRingFile=/Users/romseygeek/.gnupg/secring.gpg > signing.gnupg.executable=gpg > > This worked last time I did a release. Does anybody know if anything has > changed in gradle that means I need to change the properties file, or have > any other ideas? > > > On 27 Apr 2023, at 10:54, Alan Woodward wrote: > > > > I have started a release note here: > https://cwiki.apache.org/confluence/display/LUCENE/Release+Notes+9.6 > > > >> On 27 Apr 2023, at 09:45, Alan Woodward wrote: > >> > >> I have successfully wrestled Jenkins into submission, and there are now > 9.6 jobs for Artifacts, Check and NightlyTests. > >> > >>> On 26 Apr 2023, at 16:53, Alan Woodward wrote: > >>> > >>> NOTICE: > >>> > >>> Branch branch_9_6 has been cut and versions updated to 9.7 on stable > branch. > >>> > >>> Please observe the normal rules: > >>> > >>> * No new features may be committed to the branch. > >>> * Documentation patches, build patches and serious bug fixes may be > >>> committed to the branch. However, you should submit all patches you > >>> want to commit as pull requests first to give others the chance to > review > >>> and possibly vote against them. Keep in mind that it is our > >>> main intention to keep the branch as stable as possible. > >>> * All patches that are intended for the branch should first be > committed > >>> to the unstable branch, merged into the stable branch, and then into > >>> the current release branch. > >>> * Normal unstable and stable branch development may continue as usual. > >>> However, if you plan to commit a big change to the unstable branch > >>> while the branch feature freeze is in effect, think twice: can't the > >>> addition wait a couple more days? Merges of bug fixes into the branch > >>> may become more difficult. > >>> * Only Github issues with Milestone 9.6 > >>> and priority "Blocker" will delay a release candidate build. > >>> > >>> > >>> I am struggling to find the lucene Jenkins jobs on the new apache > build server at https://jenkins-ccos.apache.org/ - if anybody has any > hints as to how to navigate the helpful new interface with a non-functional > search box, I would be very grateful… > >>> > >>> It’s a holiday weekend coming up in the UK, so my plan is to give > Jenkins a few days to chew things over (once I actually get the jobs > running) and then build a RC on Tuesday 2nd May. > >> > > > > > - > To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org > For additional commands, e-mail: dev-h...@lucene.apache.org > >
Re: New branch and feature freeze for Lucene 9.6.0
I am fighting with gradle and GPG yet again… Gradle fails when trying to sign artefacts with the message "Cannot perform signing task ':lucene:distribution:signReleaseArchives' because it has no configured signatory”. I have GPG configured in ~/.gradle/gradle.properties as follows: org.gradle.caching=true signing.keyId= signing.secretKeyRingFile=/Users/romseygeek/.gnupg/secring.gpg signing.gnupg.executable=gpg This worked last time I did a release. Does anybody know if anything has changed in gradle that means I need to change the properties file, or have any other ideas? > On 27 Apr 2023, at 10:54, Alan Woodward wrote: > > I have started a release note here: > https://cwiki.apache.org/confluence/display/LUCENE/Release+Notes+9.6 > >> On 27 Apr 2023, at 09:45, Alan Woodward wrote: >> >> I have successfully wrestled Jenkins into submission, and there are now 9.6 >> jobs for Artifacts, Check and NightlyTests. >> >>> On 26 Apr 2023, at 16:53, Alan Woodward wrote: >>> >>> NOTICE: >>> >>> Branch branch_9_6 has been cut and versions updated to 9.7 on stable branch. >>> >>> Please observe the normal rules: >>> >>> * No new features may be committed to the branch. >>> * Documentation patches, build patches and serious bug fixes may be >>> committed to the branch. However, you should submit all patches you >>> want to commit as pull requests first to give others the chance to review >>> and possibly vote against them. Keep in mind that it is our >>> main intention to keep the branch as stable as possible. >>> * All patches that are intended for the branch should first be committed >>> to the unstable branch, merged into the stable branch, and then into >>> the current release branch. >>> * Normal unstable and stable branch development may continue as usual. >>> However, if you plan to commit a big change to the unstable branch >>> while the branch feature freeze is in effect, think twice: can't the >>> addition wait a couple more days? Merges of bug fixes into the branch >>> may become more difficult. >>> * Only Github issues with Milestone 9.6 >>> and priority "Blocker" will delay a release candidate build. >>> >>> >>> I am struggling to find the lucene Jenkins jobs on the new apache build >>> server at https://jenkins-ccos.apache.org/ - if anybody has any hints as to >>> how to navigate the helpful new interface with a non-functional search box, >>> I would be very grateful… >>> >>> It’s a holiday weekend coming up in the UK, so my plan is to give Jenkins a >>> few days to chew things over (once I actually get the jobs running) and >>> then build a RC on Tuesday 2nd May. >> > - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
