-
-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[v32.0-alpha] answering quests sometimes really slow #2803
Comments
Can you show the log? |
Answered crossing type (slow), opening hours (fast), building type (slow)
|
I took the liberty to edit that log to only show the relevant parts. The most relevant parts are:
So, disclaimer: v32.0 is more slow and more demanding on resources. Because this is what happens:
The reason why it is fast for For The main reason that it takes long for your device seem to be some memory constraints regarding the |
The map data geometry is fetched from the database using a bounding box. There is no index on the min latitude, min longitude, max latitude and max longitude of each geometry in the element geometry table because I am not sure if an index would help on a select statement like I created a little kotlin script that outputs a fun main() {
File("test.sql").printWriter().use { out ->
out.println("BEGIN TRANSACTION;")
out.println("""
CREATE TABLE "test" (
"i" INTEGER NOT NULL,
"xmin" NUMERIC NOT NULL,
"ymin" NUMERIC NOT NULL,
"xmax" NUMERIC NOT NULL,
"ymax" NUMERIC NOT NULL,
PRIMARY KEY("i" AUTOINCREMENT)
);
""".trimIndent())
for(i in 0..500000) {
out.println("INSERT INTO test (xmin,ymin,xmax,ymax) values (${Random.nextDouble()},${Random.nextDouble()},${Random.nextDouble()},${Random.nextDouble()});")
}
out.println("COMMIT TRANSACTION;")
}
} On my computer, doing a query on a database created from that SELECT * FROM test WHERE xmin >= 0.1 AND xmax <= 0.2 AND ymin >= 0.1 AND ymax <= 0.2 takes about 100ms. If I add an index like this CREATE INDEX minmax on test (xmin, xmax, ymin, ymax) it takes about 50ms. So, an index in SQLite also works with However, this is likely not the main problem, the main problem seem to be some contraints on the cursor window. Will look into that next. |
Thanks for the explanation. What I wonder: why is this lookup much faster
|
Maybe the fetching from DB (=creating of the objects) itself is somehow hugely inefficient |
I pushed the creations of the indexes to master. I'd be interested to know how much this speeds this up for you. (You need to uninstall and reinstall the app.) If possible, it would be great if you used the the exact same element for the test. |
Did a random test myself. The times are still really slow:
|
Did some more detailed performance measuring (after indexing applied) at that location. Of the total time spent:
So, SQL efficiency is not really the problem. Or if it is, it's in the iterating the SQL cursor, which can't be made any faster. Most time (83%) is spent in application code: instantiating the data. Of these a third alone is spent on deserializing the element tags. Since the data classes will be exchanged with pure kotlin ones and the serialization library is exchanged with kotlinx-serialization which is not based on Java Reflection but on the other hand serializes to a less compact format (JSON), the measurements may be different on that branch. Will take another measurement on that branch. |
Here is a log with the latest change: Log
|
so, no difference |
I have been doing performance testings all day long, but it is really hard because the times measured are quite elusive. For example, if I do one and the same thing - answer that a house is a detached (this one: https://www.openstreetmap.org/way/58502870#map=19/53.54687/9.99737) (then undo), then answer that again etc, the times measured vary from 300ms to 90ms at one and the same location. Though, what is consistent is that the first fetching takes longest, the second is shorter and then usually on the third call, it's consistently down to 90ms. I also checked if the |
Also, the times measured on what is taking long are always constant, in the sense that if the call takes 300ms, everything |
So I am out of ideas. 80ms is good, 300ms is... okay. 40000ms is unusable. Though that particular explosion of time it takes to fetch a few hundred elements from the database seems to be only reproducible on your device. I mean, when quests are displayed, a couple of thousand quests are fetched from the database - that doesn't seem to be a problem on your device? Or is your device trying to fetch this data from database for 5 minutes and during that time, any other larger database operation will also take such an amount of time? Anyway, I fear to advance with this issue, I rely on your help. You yourself must measure the operation to find what particular element of it takes this much time. |
Something similar happened to me when I started testing new version (freeze of app for several seconds on download), then failed to appear again. So I decided to blame phone for being Xiaomi-quality. I initially got scared that new code is causing massive slowdowns then it failed to appear ever again. |
hmmm 😟 |
Well, definitely the DB size that is important, as (so far) it only happens if I download many elements at once. I'm currently trying to split the download into smaller areas, hoping that it might cause geographically close elements to be "closer" in the DB (might be obviously useless; I have no clue how such a DB is organized internally and simply imagine a huge spreadsheet) |
Hmm, this is not really helping. |
No, I don't get these messages.
I think the best way would be if you first measured and logged the time it takes to fetch the geometry, the elements etc from DB
Am 27. April 2021 07:12:03 MESZ schrieb Helium314 ***@***.***>:
…> Anyway, I fear to advance with this issue, I rely on your help. You
yourself must measure the operation to find what particular element of
it takes this much time.
Well, definitely the DB size that is important, as (so far) it only
happens if I download many elements at once.
Do you (or anyone lese) get any of these "Window is full" messages
after creating a 25+ MB DB at initial download?
I'm currently trying to split the download into smaller areas, hoping
that it might cause geographically close elements to be "closer" in the
DB (might be obviously useless; I have no clue how such a DB is
organized internally and simply imagine a huge spreadsheet)
--
You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub:
#2803 (comment)
--
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.
|
Splitting the download into 16 parts decreases the fetch time to 5 sec, but only fetches 50 elements instead of 100+ (not sure how relevant this is, the number is different every time). |
What download? This ticket is about that answering quests is really slow, isn't it? That has nothing to do with the download. |
This is extremely relevant though, as this might point to another bug. Maybe log the BBOX the fetch is being made for. |
As said, please don't blindly try around such tings, first step should be what part of the process is the one thing that takes long. Then we can optimize. |
As written above: this problem appears inside large areas downloaded at once (in a single download). Not for any specific element. If I only download a tiny area at full zoom, answering the quest for the same element is done in about a second. |
retry after reboot: 39.6 sec |
Okay, let's not pursue that further. IMO it does not make a big difference whether to store it as a byte or as a string like "WAY" |
on the Do you want to try it out? There is still something to do, but I first wanted to check if it makes any difference. What's left to do if this works out:
Also, the performance problem currently regarding the huge relations should also lead to very very slow download too, because also all geometry in the bbox a new download happens is fetched from DB (so also those huge relations). This change also addresses that. |
Using |
Heh, so it's 300 times faster?
Am 1. Mai 2021 08:30:08 MESZ schrieb Helium314 ***@***.***>:
…Using `fetch-by-bbox` fetches 17 elements in 150ms for the same house,
so definitely a difference.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2803 (comment)
--
Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.
|
Those 17 (instead of 100+) elements probably don't contain any of those huge relations... |
Though 17 sound too little. JOSM fetches 29 |
Just as a side note, that particular area is really problematic because there are so many huuuuuuge relations. Download of that area for me was like:
And that is nothing this app can do anything about. Relations are included in the download result. It would be possible to throw away relations that are huge route relations after download. That would decrease the time used to persist and later also to query the data back from the db. |
The data returned is http://overpass-turbo.eu/s/16Rk
So that looks correct to me. It takes 172ms on the emulator. |
And if I look for elements in a larger bbox (bbox of house + 40m radius), we already got some pretty large relations in the result: http://overpass-turbo.eu/s/16Rn
This takes 1211ms on my emulator. This time can likely be halved when doing that performance improvement mentioned earlier. |
So anyway, if you solved a quest over at Südosttangente where all the international bus routes run through, the performance problem would be back. So maybe the better solution (or the one that be done additionally) is to really exclude all relations except a few chosen ones to be persisted. The effects of this would be that the summit register quest needs to be deleted. It is not a big sacrifice IMO. Could it have any other negative effects? For example when a road is split, potentially all relations the way runs through needs to be updated. If the relations are not available offline, the split would be done "wrong". Well, I think not, because the local split is not connected with the "actual split" that is done when uploading the split. When uploading the split, the algorithm actually queries "give me all relations that reference this way" directly from the OSM API and works with that. @matkoniecz which are the relation types that should be kept? |
Definitely worth other improvements and avoiding insane performance issues. And maybe it can get back somehow. The only issue is that it can be potentially triggering some problems in achievement counter, maybe in settings (but it is not the first removed/renamed quest, so...).
What about fetching all relation, but without recursive fetching of their elements? Otherwise associatedStreet is needed but fetching just elements in bbox is fine and ferry quests may want ferry routes - but long ones are discarded anyway, and short ones are unlikely to be mapped as relations... |
That was never done. It is the fetching of huge relations alone that is creating these problems. |
Or maybe a negative list? In that negative list, there should be at least network, water, superroute, boundary, route, route_master, waterway. |
What about a small amount of denormalizing? For relations in the DB, also store the number of (known) members, and queries can add a filter to exclude relations of a certain size. Then the summit quest (and any similar quests in the future) could run async in the background, without this restriction (so they may take a while to appear, but that's fine). |
2 tests, for "that house" and a crossing next to bus terminal, below A23 old fetch-by-bbox (download all relations): 17 elements in 138 ms, 113 elements in 44500 ms Regarding routes: Alternatively, how about not discarding hiking/ferry routes at all? Or only those with more than 1000 members? |
Is it possible to discard sch relations without ever fetching them? ( "It is the fetching of huge relations alone that is creating these problems." #2803 (comment) ) |
Thank you for the tests, @Helium314 These two are important:
So, both discard route etc relations. The fetch-by-bbox branch must mitigate that one problem I talked about, which is why a double-padding (or similar) would make sense. With that double padding, it is clear that there is not really any advantage of the approach in the fetch-by-bbox branch, so that can be discarded. |
Ok, I also changed two more things:
|
for "our house" here. I hereby declare this performance problem solved |
And released an alpha version! I hope all the issues have been purged now. But I am not so confident about that, so please continue to report any problem here! :-) |
This is no longer important, but for reference…
Ack, I had meant to link this article that I found while reading about it, which is from 2017, but I assume still applies: https://medium.com/androiddevelopers/large-database-queries-on-android-cb043ae626e8
This was also my take-away from the article, and that the cursor windows may overlap (when you move to a position, it starts filling the window before that position, not at the position). Also that you can adjust that behavior and the cursor size on api 28+, but earlier than that the only way to optimize is a custom implementation. |
But tis does not occur for StreetComplete. Even a relation with 1000000 members would not trigger that it does not fit into one CursorWindow because each relation member is one row in the database table. |
Only the problem that one row doesn't fit into the CursorWindow can't occur. But the problem is that the entire set of results (mostly rows of the relation members table) is larger than the CursorWindow. Additional useful link/answer: https://stackoverflow.com/questions/20094421/cursor-window-window-is-full/52505989#52505989 I actually wanted to increase the CursorWindow size (possible on Android 9+) to test how this affects speed, but I don't understand how to actually do this. |
Maybe the RAM is not enough and there is some swapping going on? Otherwise, no idea. |
I have not done it myself, but it looks something like this:
It seems like this still requires replacing the cursor window after the first query has already been run (ie, refreshing the cursor using the new window), but if it was filling up several times then this should still be an improvement. |
After downloading a large area containing many quests, answering some quests takes very long (in my case this was done automatically after first app start, but also happens when zooming out and starting a manual scan). Especially for building or crossing quests, on my S4 Mini it takes some 30 seconds to for the pin to be removed. In this time I can't open any other quests, SC usage is basically blocked (panning/zooming the map is still possible without noticable slowdowns).
Other quests like opening hours do not have this problem.
Interestingly, answering quests near the southern edge of the scanned area is much faster than in other regions.
When "leaving" this area, i.e. scrolling somewhere far away, manually scanning and answering quests is fast (less than a second). The southern edge of the new area might be a little bit faster, not sure.
Sometimes after doing stuff "somewhere else", anwering quests inside the initial scan area is faster (maybe 10 seconds instead of 30 for the same element), but still slower than outside. I haven't yet found a way of reproducing this.
The text was updated successfully, but these errors were encountered: