You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/09/27 07:57:29 UTC
[GitHub] [lucene] iverase opened a new issue, #11824: Performance regression on LatLonPoint#newPolygonQuery
iverase opened a new issue, #11824:
URL: https://github.com/apache/lucene/issues/11824
### Description
I just notice a big performance regression on polygon queries using LatLonPoint field in [lucene geo benchmarks](https://home.apache.org/~mikemccand/geobench.html):
<img width="1765" alt="image" src="https://user-images.githubusercontent.com/29038686/192466543-419575d8-e1c2-483c-81e4-c122a92a694f.png">
I checked and the regression was introduced by this change: https://github.com/apache/lucene/pull/1017.
My suspicion is that before this change, SpatialQuery was calling the method `#getSpatialVisitor()` once for the whole index but in the new version is calling it once per segment. This method might be expensive for LatLonPoint queries, threfore the regression.
@nknize FYI
### Version and environment details
_No response_
--
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-unsubscribe@lucene.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase commented on issue #11824: Performance regression on LatLonPoint#newPolygonQuery
Posted by GitBox <gi...@apache.org>.
iverase commented on issue #11824:
URL: https://github.com/apache/lucene/issues/11824#issuecomment-1260667934
Fix seems to bring performance back to previous levels:
<img width="1380" alt="image" src="https://user-images.githubusercontent.com/29038686/192749125-0ac0b341-b2cb-4395-991c-9f676322592a.png">
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] nknize commented on issue #11824: Performance regression on LatLonPoint#newPolygonQuery
Posted by GitBox <gi...@apache.org>.
nknize commented on issue #11824:
URL: https://github.com/apache/lucene/issues/11824#issuecomment-1259733493
Just seeing this. That's exactly what it would be! Snuck in one of those last commits on the long running PR. Thanks for refactoring and merging @iverase!
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] nknize commented on issue #11824: Performance regression on LatLonPoint#newPolygonQuery
Posted by GitBox <gi...@apache.org>.
nknize commented on issue #11824:
URL: https://github.com/apache/lucene/issues/11824#issuecomment-1260938633
What's annoying is how incredibly trappy this override logic is. That a method call literally moving from `createWeight` to `getScorerSupplier` results in a 72.2% regression even slipped by me before merge doesn't bode well for new committers. And then it sat in regression until an entire company was interested in releasing.
I wonder if we can do better? Like maybe figure out better guardrails in these methods? Perhaps by something as simple as a rename to signal one happens per segment? This isn't the first and will certainly not be the last time an expensive operation accidentally slips to a critical path. Any other ideas how to lower the bar here for new committers?
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase commented on issue #11824: Performance regression on LatLonPoint#newPolygonQuery
Posted by GitBox <gi...@apache.org>.
iverase commented on issue #11824:
URL: https://github.com/apache/lucene/issues/11824#issuecomment-1259240627
close in https://github.com/apache/lucene/pull/11825
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mikemccand commented on issue #11824: Performance regression on LatLonPoint#newPolygonQuery
Posted by GitBox <gi...@apache.org>.
mikemccand commented on issue #11824:
URL: https://github.com/apache/lucene/issues/11824#issuecomment-1260787630
Thanks for catching this @iverase and the quick fix, and the follow-on issue to better detect such regressions before release: #11827
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] iverase closed issue #11824: Performance regression on LatLonPoint#newPolygonQuery
Posted by GitBox <gi...@apache.org>.
iverase closed issue #11824: Performance regression on LatLonPoint#newPolygonQuery
URL: https://github.com/apache/lucene/issues/11824
--
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-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org