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

[WIP] Speed up geojson output #537

Closed
wants to merge 1 commit into from

Conversation

Algunenano
Copy link
Member

Trac issue: https://trac.osgeo.org/postgis/ticket/4615

Printing the doubles directly into the output buffer doubles the tps with big inputs:

Before 02e62ee:

number of clients: 1
number of threads: 1
duration: 30 s
number of transactions actually processed: 31
latency average = 975.931 ms
tps = 1.024663 (including connections establishing)
tps = 1.024788 (excluding connections establishing)

After 02e62ee:

number of clients: 1
number of threads: 1
duration: 30 s
number of transactions actually processed: 58
latency average = 518.217 ms
tps = 1.929693 (including connections establishing)
tps = 1.929935 (excluding connections establishing)

After the introduction of Ryu (https://trac.osgeo.org/postgis/ticket/4543) there are new rough corners, so I plan to keep polishing for a while before merging.

@Komzpa
Copy link
Member

Komzpa commented Jan 22, 2020

❤️

@Algunenano
Copy link
Member Author

Some notes about other things I've tested:

  • I've removed the need to calculate the length of the final string (aka, use cstring_to_text_with_len instead of cstring_to_text) by passing the size to lwgeom_to_geojson as an output value: Initially I introduced a bug and the size returned was the max calculated size, not the actual output size which made 6% slower and left me surprised; after fixing it: 1% faster. Not worth the trouble.

  • As mentioned in https://trac.osgeo.org/postgis/ticket/4623, I've tried to create a new liblwgeom type that contains both the size and the data, that is, a varlena compatible type so there is no need to transform the data from liblwgeom to Postgresql: 7% improvement, might be worth the trouble even if it's just for geojson (but I expect the impact on other functions to be much higher since the time spent on the function bodies is smaller, e.g St_AsBinary): Algunenano@6f86f18

After the previous change (the varlena type) the current process with big geometries show an ussage like this:

+   98.53%     0.01%             2  postgres  postgis-3.so            [.] LWGEOM_asGeoJson                                                                                                                                                                  ▒
+   94.78%     0.02%            45  postgres  postgis-3.so            [.] lwgeom_to_geojson                                                                                                                                                                 ▒
+   94.63%     0.04%            65  postgres  postgis-3.so            [.] asgeojson_multipolygon_buf                                                                                                                                                        ▒
+   94.41%     4.05%          4720  postgres  postgis-3.so            [.] pointArray_to_geojson                                                                                                                                                             ▒
+   90.51%     7.85%          8992  postgres  postgis-3.so            [.] lwprint_double                                                                                                                                                                    ▒
+   82.84%    80.38%         90489  postgres  postgis-3.so            [.] d2fixed_buffered_n

So technically, between the 98.53% for LWGEOM_asGeoJson and the time spent printing doubles (which should be the high as possible), that is 82.84%, there is only 16% of time being spent, which is low. I might still have a look to see if anything can be improved, but I might end up pushing only this commit here and the varlena change as part of a different PR implementing it for other functions.

@pramsey
Copy link
Member

pramsey commented Jan 24, 2020

Great work @Algunenano 👍

@strk strk closed this in 892fcde Jan 27, 2020
@Algunenano Algunenano deleted the geojson_speedup branch January 27, 2020 10:16
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