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

Improve performance of JSON rendering #6380

Merged
merged 29 commits into from
Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
463663b
wip
SiarheiFedartsou Sep 28, 2022
a9c72e3
Refactor json renderer to share implementation for vector and string
SiarheiFedartsou Sep 30, 2022
cb0af6b
Add string
SiarheiFedartsou Sep 30, 2022
4b0ea28
Merge branch 'master' into sf-json-perf
SiarheiFedartsou Sep 30, 2022
c784ff7
Optimize strings
SiarheiFedartsou Sep 30, 2022
b9957bb
Get rid of copy
SiarheiFedartsou Sep 30, 2022
3781068
add check
SiarheiFedartsou Sep 30, 2022
666ff1c
use std::string in node bindings
SiarheiFedartsou Sep 30, 2022
d2ce6d2
add comments
SiarheiFedartsou Sep 30, 2022
ca7e560
fix bug
SiarheiFedartsou Sep 30, 2022
05c6008
add comments
SiarheiFedartsou Sep 30, 2022
4f222be
add comments
SiarheiFedartsou Sep 30, 2022
889e4ce
add comments
SiarheiFedartsou Sep 30, 2022
87db480
add comments
SiarheiFedartsou Sep 30, 2022
258d656
fix bug
SiarheiFedartsou Oct 1, 2022
a4d1783
use fmt
SiarheiFedartsou Oct 1, 2022
ee7499d
use fmt
SiarheiFedartsou Oct 1, 2022
d033e3b
use fmt
SiarheiFedartsou Oct 1, 2022
d871e20
use fmt
SiarheiFedartsou Oct 1, 2022
bc6a4d8
add test
SiarheiFedartsou Oct 1, 2022
ae157de
add test
SiarheiFedartsou Oct 1, 2022
6a5b3f4
add test
SiarheiFedartsou Oct 1, 2022
50efe45
update changelog
SiarheiFedartsou Oct 1, 2022
04429d4
update
SiarheiFedartsou Oct 1, 2022
7430d62
optimize nodejs
SiarheiFedartsou Oct 1, 2022
03f9671
update
SiarheiFedartsou Oct 1, 2022
32deb11
update benchmark
SiarheiFedartsou Oct 2, 2022
cbc0868
update benchmark
SiarheiFedartsou Oct 2, 2022
f057ef7
update benchmark
SiarheiFedartsou Oct 3, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/osrm-backend.yml
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,7 @@ jobs:
pushd ${OSRM_BUILD_DIR}
Copy link
Member Author

@SiarheiFedartsou SiarheiFedartsou Sep 30, 2022

Choose a reason for hiding this comment

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

0.

Baseline on my M1 Pro/32 gb ram:
initial

Copy link
Member Author

Choose a reason for hiding this comment

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

"string" here is std::stringstream ss; ... = ss.str()

make --jobs=${JOBS} benchmarks
./src/benchmarks/alias-bench
./src/benchmarks/json-render-bench
./src/benchmarks/match-bench ../test/data/ch/monaco.osrm
./src/benchmarks/packedvector-bench
./src/benchmarks/rtree-bench ../test/data/monaco.osrm.ramIndex ../test/data/monaco.osrm.fileIndex ../test/data/monaco.osrm.nbg_nodes
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- NodeJS:
- FIXED: Support `skip_waypoints` in Node bindings [#6060](https://github.com/Project-OSRM/osrm-backend/pull/6060)
- Misc:
- CHANGED: Improve performance of JSON rendering. Fix undefined behaviour in JSON numbers formatting. [#6380](https://github.com/Project-OSRM/osrm-backend/pull/6380)
- ADDED: Add timestamps for logs. [#6375](https://github.com/Project-OSRM/osrm-backend/pull/6375)
- CHANGED: Improve performance of map matching via getPathDistance optimization. [#6378](https://github.com/Project-OSRM/osrm-backend/pull/6378)
- CHANGED: Optimize RestrictionParser performance. [#6344](https://github.com/Project-OSRM/osrm-backend/pull/6344)
Expand Down
2 changes: 1 addition & 1 deletion features/testbot/annotations.feature
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,4 @@ Feature: Annotations

When I route I should get
| from | to | route | a:speed | a:distance | a:duration | a:nodes |
| a | c | abc,abc | 10:10 | 249.987619:299.962882 | 25:30 | 1:2:3 |
| a | c | abc,abc | 10:10 | 249.987618946:299.962882039 | 25:30 | 1:2:3 |
Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to be expected since we changed algorithm used for formatting.

6 changes: 3 additions & 3 deletions features/testbot/matching.feature
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ Feature: Basic Map Matching

# These should have the same weights/duration in either direction
When I match I should get
| trace | geometry | a:distance | a:duration | a:weight | duration |
| 2345 | 1.00018,1,1.000314,1 | 14.914666 | 1.4 | 1.4 | 1.4 |
| 4321 | 1.00027,1,1.000135,1 | 15.02597 | 1.5 | 1.5 | 1.5 |
| trace | geometry | a:distance | a:duration | a:weight | duration |
| 2345 | 1.00018,1,1.000314,1 | 14.914666491 | 1.4 | 1.4 | 1.4 |
| 4321 | 1.00027,1,1.000135,1 | 15.025969972 | 1.5 | 1.5 | 1.5 |

4 changes: 2 additions & 2 deletions features/testbot/snap_intersection.feature
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ Feature: Snapping at intersections
| a,f,k | ac,cf,cf,fj,kj,kj | 132.8s | 132.8 |
| k,f | ik,fi,fi | 54.3s | 54.3 |
| f,a | ef,ae,ae | 66.6s | 66.6 |
| k,f,a | kj,fj,fj,ef,ae,ae | 141.4s | 141.4 |
| k,f,a | kj,fj,fj,ef,ae,ae | 141.399999999s | 141.399999999 |

When I request a travel time matrix I should get
| | a | f | k |
Expand Down Expand Up @@ -626,4 +626,4 @@ Feature: Snapping at intersections
| a,f,k | ad,df,df,fj,kj,kj | 105.6s | 105.6 |
| k,f | ik,fi,fi | 54.3s | 54.3 |
| f,a | ef,ae,ae | 66.6s | 66.6 |
| k,f,a | ik,fi,fi,ef,ae,ae | 120.9s | 120.9 |
| k,f,a | ik,fi,fi,ef,ae,ae | 120.899999999s | 120.899999999 |
10 changes: 5 additions & 5 deletions features/testbot/weight.feature
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ Feature: Weight tests
| abc |

When I route I should get
| waypoints | route | distances | weights | times | a:distance | a:duration | a:weight | a:speed |
| s,t | abc,abc | 20m,0m | 2,0 | 2s,0s | 20.034627 | 2 | 2 | 10 |
| t,s | abc,abc | 20m,0m | 2,0 | 2s,0s | 20.034627 | 2 | 2 | 10 |
| s,e | abc,abc | 40m,0m | 3.9,0 | 3.9s,0s | 29.940636:10.017313 | 3:0.9 | 3:0.9 | 10:11.1 |
| e,s | abc,abc | 40m,0m | 3.9,0 | 3.9s,0s | 10.017313:29.940636 | 0.9:3 | 0.9:3 | 11.1:10 |
| waypoints | route | distances | weights | times | a:distance | a:duration | a:weight | a:speed |
| s,t | abc,abc | 20m,0m | 2,0 | 2s,0s | 20.034626629 | 2 | 2 | 10 |
| t,s | abc,abc | 20m,0m | 2,0 | 2s,0s | 20.034626629 | 2 | 2 | 10 |
| s,e | abc,abc | 40m,0m | 3.9,0 | 3.9s,0s | 29.940636463:10.017313314 | 3:0.9 | 3:0.9 | 10:11.1 |
| e,s | abc,abc | 40m,0m | 3.9,0 | 3.9s,0s | 10.017313314:29.940636463 | 0.9:3 | 0.9:3 | 11.1:10 |


Scenario: Step weights -- way_function: fail if no weight or weight_per_meter property
Expand Down
5 changes: 3 additions & 2 deletions fuzz/escape_json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
#include <iterator>
#include <string>

using osrm::util::escape_JSON;
using osrm::util::EscapeJSONString;

extern "C" int LLVMFuzzerTestOneInput(const unsigned char *data, unsigned long size)
{
const std::string in(reinterpret_cast<const char *>(data), size);

const auto escaped = escape_JSON(in);
std::string escaped;
EscapeJSONString(in, escaped);
escape(escaped.data());

return 0;
Expand Down
4 changes: 2 additions & 2 deletions include/nodejs/json_v8_renderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ struct V8Renderer

inline void renderToV8(v8::Local<v8::Value> &out, const osrm::json::Object &object)
{
osrm::json::Value value = object;
Copy link
Member Author

Choose a reason for hiding this comment

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

This copy is expensive...

mapbox::util::apply_visitor(V8Renderer(out), value);
V8Renderer renderer(out);
renderer(object);
}
} // namespace node_osrm

Expand Down
49 changes: 0 additions & 49 deletions include/util/cast.hpp

This file was deleted.

Loading