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

Use ujson for serializing GeoJSON and TopoJSON #143

Merged
merged 1 commit into from
Jan 6, 2017
Merged

Conversation

iandees
Copy link
Member

@iandees iandees commented Jan 6, 2017

For #139.

simplejson was merged into Python core in 2.6, so there's actually a slow-down using simplejson 3.6.4 (which is currently installed, but unused) or 3.10.0 (the latest version):

Coord Format Python 2.7.6
Built-in
simplejson 3.6.4 simplejson 3.10.0
1/1/1 json 358.7ms 385.3 (+7.42%) 387.1 (+7.93%)
1/1/1 topojson 232.0ms 252.9ms (+8.98%) 257.3 (+10.91%)
7/32/47 json 897.2 910.4ms (+1.46%) 927.5 (+3.37%)
7/32/47 topojson 567.7 586.6ms (+3.33%) 601.3 (+5.92%)
12/1050/1522 json 3238.7 3366.8ms (+3.95%) 3368.8 (+4.02%)
12/1050/1522 topojson 2128.4 2213.2ms (+3.98%) 2244.2 (+5.44%)
16/16812/24357 json 390.2 395.9ms (+1.45%) 412.5 (+5.70%)
16/16812/24357 topojson 296.6 294.3ms (-0.79%) 294.7 (-0.63%)

As a result, this pull request switches to use ujson, a faster (but less feature-rich) JSON library. It seems to shave 10-20% of time:

Coord Format Python 2.7.6
Built-in
ujson 1.35
1/1/1 json 358.7ms 292.7ms (-18.4%)
1/1/1 topojson 232.1ms 189.1ms (-18.5%)
7/32/47 json 897.3ms 780.9ms (-13%)
7/32/47 topojson 567.7ms 488.9ms (-13.9%)
12/1050/1522 json 3238.8ms 2935ms (-9.4%)
12/1050/1522 topojson 2128.5ms 1858.7ms (-12.7%)
16/16812/24357 json 390.3ms 334.3ms (-14.3%)
16/16812/24357 topojson 296.7ms 240.5ms (-18.9%)

ujson resulted in a < 2% difference for the mvt and mvtb formats.

@nvkelso
Copy link
Member

nvkelso commented Jan 6, 2017

Nice work! I was expecting a similar (or slightly smaller) improvement in MVT generation since it first has to calculate the JSON version. Does that mean most the savings here is JSON > String > file IO related changes?

@nvkelso
Copy link
Member

nvkelso commented Jan 6, 2017

Please also link up related PR for tileserver changes.

@iandees
Copy link
Member Author

iandees commented Jan 6, 2017

My measurements here are for the runtime of the _create_formatted_tile() function for each of the 4 formats, and it doesn't look like the mvt format does anything with JSON at all.

@iandees
Copy link
Member Author

iandees commented Jan 6, 2017

@nvkelso said:

Please also link up related PR for tileserver changes.

A tileserver PR would just be an adjustment of the tilequeue dependency, I think. We'd need to merge this PR and tag tilequeue first.

@nvkelso
Copy link
Member

nvkelso commented Jan 6, 2017

We declare simplejson in the dependencies for tileserver now (which seems weird to me, but @rmarianski said it was to play nicer with tilequeue?):

And we import json in __init__.py:

@nvkelso
Copy link
Member

nvkelso commented Jan 6, 2017

To observe the expected MVT format times improvement, I suspect you'd additionally need to measure the total time tileserver took to generate the example tiles per format (not just the _create_formatted_tile() function in tilequeue)?

@iandees
Copy link
Member Author

iandees commented Jan 6, 2017

Yep, it looks like tileserver does the JSON decoding stuff here.

@rmarianski
Copy link
Member

We declare simplejson in the dependencies for tileserver now (which seems weird to me, but @rmarianski said it was to play nicer with tilequeue?):

It should be removed from all requirements files.

Yep, it looks like tileserver does the JSON decoding stuff here.

Some of that, if not all, was a vestige of when tileserver handled custom layer requests. There's an issue for cleaning that up.

@iandees
Copy link
Member Author

iandees commented Jan 6, 2017

I checked before and after ujson and it looks like the only difference is that the default output for ujson is to not include and whitespace between JSON syntax. For example, ujson outputs:

"landuse":{"type":"FeatureCollection","features":[]}}

whereas the built-in JSON serializer includes whitespace:

"landuse": {"type": "FeatureCollection", "features": []}}

@iandees iandees merged commit 98aa5c8 into master Jan 6, 2017
@iandees iandees removed the in review label Jan 6, 2017
@iandees iandees deleted the use_ujson branch October 10, 2017 18:33
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