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

Switch to using ujson, remove simplejson from requirements #56

Merged
merged 1 commit into from
Jan 6, 2017

Conversation

iandees
Copy link
Member

@iandees iandees commented Jan 6, 2017

Related to tilezen/tilequeue#143 and tilezen/tilequeue#139.

Switch to using ujson, a faster JSON library, when loading the JSON tile to rebuild into a different format.

@nvkelso
Copy link
Member

nvkelso commented Jan 6, 2017

LGTM

@iandees
Copy link
Member Author

iandees commented Jan 6, 2017

I'm pretty sure we need to point to a new tagged version of tilequeue in this PR, right?

I was also working on setting up a profiling for tileserver.

@nvkelso nvkelso changed the title Switch to using ujson, remove simplejson from requirements WIP: Switch to using ujson, remove simplejson from requirements Jan 6, 2017
@rmarianski
Copy link
Member

I'm pretty sure we need to point to a new tagged version of tilequeue in this PR, right?

Wrt the requirements txt file checked into the repo? If so, I think it should just be left out.

@iandees
Copy link
Member Author

iandees commented Jan 6, 2017

The change on tileserver isn't nearly as impactful:

Coord Format Python 2.7.6
Built-in
ujson 1.3.5
1/1/1 json 290.2ms 300ms (3.4%)
1/1/1 topojson 233.4ms 213.8ms (-8.4%)
1/1/1 mvt 807.5ms 817.5ms (1.2%)
1/1/1 mvtb 814.9ms 873.9ms (7.2%)
7/32/47 json 715.9ms 689.7ms (-3.7%)
7/32/47 topojson 470.5ms 453.3ms (-3.7%)
7/32/47 mvt 1091.9ms 1055.4ms (-3.3%)
7/32/47 mvtb 1165.2ms 1115.3ms (-4.3%)
12/1050/1522 json 2207ms 2172.4ms (-1.6%)
12/1050/1522 topojson 1605.1ms 1654.1ms (3.1%)
12/1050/1522 mvt 3729.4ms 3667.7ms (-1.7%)
12/1050/1522 mvtb 3876ms 3515.9ms (-9.3%)
16/16812/24357 json 286.5ms 295.6ms (3.2%)
16/16812/24357 topojson 208.9ms 206.6ms (-1.1%)
16/16812/24357 mvt 559.1ms 500ms (-10.6%)
16/16812/24357 mvtb 523.3ms 571.7ms (9.2%)

@nvkelso
Copy link
Member

nvkelso commented Jan 6, 2017

Still looks overall better, so let's proceed with merging and testing on dev.

@iandees iandees merged commit 528d4f4 into master Jan 6, 2017
@iandees iandees deleted the use_ujson branch January 6, 2017 20:44
@iandees iandees removed the in review label Jan 6, 2017
@iandees iandees changed the title WIP: Switch to using ujson, remove simplejson from requirements Switch to using ujson, remove simplejson from requirements Jan 6, 2017
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.

5 participants