[SIMBAD] Improve query region preformances#3235
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3235 +/- ##
==========================================
+ Coverage 68.69% 69.07% +0.38%
==========================================
Files 231 232 +1
Lines 19214 19617 +403
==========================================
+ Hits 13199 13551 +352
- Misses 6015 6066 +51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
keflavich
left a comment
There was a problem hiding this comment.
Interesting! Thanks for these performance improvements!
|
Thanks for the super quick review! I'm a bit uncertain about not having a remote test for the longer list of centers case though. I might add one with the SKIP_SLOW tag EDIT: I added a remote test and for 360 points I get a ~0.25s test duration so I added the test without the SKIP_SLOW. |
dc765f8 to
77203fa
Compare
bsipocz
left a comment
There was a problem hiding this comment.
Two minor comments otherwise this looks all good.
Thanks!
29531a2 to
edbd6d8
Compare
edbd6d8 to
8c84b28
Compare
bsipocz
left a comment
There was a problem hiding this comment.
Thanks for addressing the comments, it's looking all good!
While trying to help with #3209 I discovered that
query_regiontimeouts for large number of centers (~500). However, the query is way faster with uploads for this case.There is a strange behavior that for smaller queries, the former method is still faster. Maybe due to the writing and then reading time for the uploaded file? It's not an exact science, but I benchmarked a little, and found that switching around 300 centers between the former method and a TAP upload gave the best result.
That way, a query with the maximum number of lines allowed by the upload method (200 000 centers) executes in ~ 1m30 for a 1 arcmin radius. Users will still have to split into smaller queries if they have more than 200 000 centers.