Use Web Workers(wip) #5180
Use Web Workers(wip) #5180
Conversation
There was a problem hiding this comment.
As a first attempt, I am trying to only parse the node, ways, relations in the web worker and then sending a simple JSON back to the main thread.
If it's just tile parsing, that would be a good first step. There are lots of places iD could offload tasks to workers... This PR seems to require changes to osm-auth, so I think it probably goes further than what you described.
| .then(function(bundle) { | ||
| return bundle.write({ | ||
| format: 'iife', | ||
| file: 'dist/iD-worker.js', |
There was a problem hiding this comment.
It's fine to generate a worker.js, but we really should leave dist/iD.js alone
There was a problem hiding this comment.
Do you mean not putting iD-worker.js to the dist folder? or renaming it worker.js?
| ); | ||
| } | ||
|
|
||
| export function authdParseBboxEntities(xhrParams, done) { |
There was a problem hiding this comment.
I don't understand why there are separate auth'd and non auth'd parsers. At the point that the worker gets involved haven't they already been downloaded?
There was a problem hiding this comment.
The worker code makes the network request to OSM. There are two possible network requests that iD wants to make, one with authentication headers and the other without it. Since the worker makes the network request and not the main iD, I made two separate worker functions to do the same.
There was a problem hiding this comment.
Yea but I thought you said you were going to start off with a version that doesn't do the request, and only does the tile parsing, since that's easier.
There was a problem hiding this comment.
Ya, I did intend to start off with that but then the passing back and forth between worker and main thread is too slow. I think letting the Webworker make the network request is the best possible way we can offload the entire task and with the current aproach, I think it is working quite fine.
|
Parsing XML line by line feels like a bit of a hack, and not very sustainable for the future. In fact breaking such a parser by stripping all whitespace is only a single line away: zerebubuth/openstreetmap-cgimap#154 (comment) To make parsing easier and faster in Javascript, we're looking into ways to enable JSON output for the OSM API /map call, so that you could get rid of XML parsing altogether. Watch this one for progress: zerebubuth/openstreetmap-cgimap#153 The format will probably be according to the Overpass API JSON format. |
|
You can try out some JSON output on the dev server for now: https://master.apis.dev.openstreetmap.org/api/0.6/map.json?bbox=-80.28218507766725,25.773537461179274,-80.24941921234132,25.782676914766654 |
|
@mmd-osm is there an expected timeline for JSON output to land to production stack? |
|
@kepta : openstreetmap/operations#244 has the latest status. The cgimap PPA package is ready for deployment. However, there are some concerns that the Rails port doesn't support JSON, and this would complicate future development. Hopefully this will get resolved very soon. |
AFAIK doing this in Rails is more-or-less automatic. |
|
Anyway we don't need to wait for the OSM JSON API to be ready to set up a webworker background task queue. I'd use it immediately for fetching lots of things.. (pretty much anything under |
I don't know what the specific issue is here. In any case it's not the first time the Rails port returns some JSON - the notes.json call already has some similar code in place: |
In case it wasn't obvious: it is ready on the dev instance, and we'd very much appreciate any sorts of feedback before JSON output hits production. So please do give it a try while it's still on dev! (I know, that's completely independent of Web Workers... it's probably better to do a follow up in #3765) |
|
related #1727 |
This brings in support for web workers in iD. The goal of this PR is to offload some work from the main iD process to a thread. As a first attempt, I am trying to only parse the node, ways, relations in the web worker and then sending a simple JSON back to the main thread.
There are a bunch of things that need to be fixed, for eg tests, renaming
idly-faster-osm-parser->osm-bizly, remove es6 code, get the changes in osm-auth merged, etc.@bhousel what do you think about the overall changes/code flow?