Skip to content
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

Process trace files once #5233

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AntonKhorev
Copy link
Collaborator

Currently uploaded gpx files are unpacked and parsed each time the points method is called. This happens a dozen of times: once for storing the trackpoints in the db, once for generating a thumbnail and once for each frame of an animated trace image. Even if we don't mind reparsing the files over and over, there's another problem. I started adding parse error handling in #5226 and I want to handle one more error (invalid characters in xml), thus I want parse errors to happen in a predictable place.

To avoid parsing files again I store trackpoint coordinates while reading each file. These coordinates are later used to draw images.

Copy link
Contributor

@nenad-vujicic nenad-vujicic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement! It works great on my side. Thank you very much!

@tomhughes
Copy link
Member

I think this winds up with quite a confused interface - you can now read multiple files into a GPX object and they get appended to the set of points which is a perfectly valid design but if you're going to go that way then I don't think read should also be calling the callback for each point.

Either the object should represent a file as before and there should be a call to iterate the points in the file, or you should be able to add multiple files and then iterate the points in one go.

@tomhughes
Copy link
Member

I was wondering about the memory overhead of this and the answer seems to be that the worse case we allow of one million points used about 11Mb for each array which seems acceptable.

@AntonKhorev
Copy link
Collaborator Author

Removed interface changes

@AntonKhorev
Copy link
Collaborator Author

I was wondering about the memory overhead

Alternatively we can do two passes. After the first pass we'll know lat/lon spans and the number of trackpoints which lets us start rendering. Then on the second pass we render the thumbnail and all animation frames in parallel. This doesn't require storing lats/lons in memory. But it may not save much in case of archived files that are read into a string. That string is likely larger than the lat/lon arrays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants