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

Optimize object merging for common cases used in Json macros #69

Merged
merged 1 commit into from Jun 3, 2017

Conversation

Projects
None yet
2 participants
@gmethvin
Member

gmethvin commented May 25, 2017

The functional combinators, which are also used by the JSON macros, suffer from some pretty serious performance issues because of the way they're constructed. For Writes, most of the time is spent constructing objects by merging JsPaths and merging in single-element objects, so I've optimized for those cases to get about a 5x speedup. I've also included some JMH benchmarks.

@gmethvin

This comment has been minimized.

Show comment
Hide comment
@gmethvin

gmethvin May 25, 2017

Member

Here are my latest results (first is using Json.writes, second is a hand-written Writes):

[info] Benchmark                                                   Mode  Cnt        Score        Error  Units
[info] JsonMacros_01_SerializeSimpleCaseClass.toJson              thrpt   20  2146153.356 ±  51624.225  ops/s
[info] JsonMacros_01_SerializeSimpleCaseClass.toJsonManualWrites  thrpt   20  3537680.745 ± 187004.753  ops/s

So about half as fast as writing the writes manually. Before the optimization it looked like this for the same test:

[info] Benchmark                                                   Mode  Cnt        Score       Error  Units
[info] JsonMacros_01_SerializeSimpleCaseClass.toJson              thrpt   20   372935.134 ± 29559.442  ops/s
[info] JsonMacros_01_SerializeSimpleCaseClass.toJsonManualWrites  thrpt   20  3475255.975 ± 87593.976  ops/s

(tests run using benchmarks/jmh:run -i 20 -wi 10 -f1 -t8)

Member

gmethvin commented May 25, 2017

Here are my latest results (first is using Json.writes, second is a hand-written Writes):

[info] Benchmark                                                   Mode  Cnt        Score        Error  Units
[info] JsonMacros_01_SerializeSimpleCaseClass.toJson              thrpt   20  2146153.356 ±  51624.225  ops/s
[info] JsonMacros_01_SerializeSimpleCaseClass.toJsonManualWrites  thrpt   20  3537680.745 ± 187004.753  ops/s

So about half as fast as writing the writes manually. Before the optimization it looked like this for the same test:

[info] Benchmark                                                   Mode  Cnt        Score       Error  Units
[info] JsonMacros_01_SerializeSimpleCaseClass.toJson              thrpt   20   372935.134 ± 29559.442  ops/s
[info] JsonMacros_01_SerializeSimpleCaseClass.toJsonManualWrites  thrpt   20  3475255.975 ± 87593.976  ops/s

(tests run using benchmarks/jmh:run -i 20 -wi 10 -f1 -t8)

@gmethvin

This comment has been minimized.

Show comment
Hide comment
@gmethvin

gmethvin May 25, 2017

Member

I should also note that the way I'm merging objects should run in amortized O(n) time instead of the O(n2) we had before (since we were actually copying the objects). And for the merging part I'm using a mutable map which should be faster than the immutable version.

Member

gmethvin commented May 25, 2017

I should also note that the way I'm merging objects should run in amortized O(n) time instead of the O(n2) we had before (since we were actually copying the objects). And for the merging part I'm using a mutable map which should be faster than the immutable version.

@marcospereira

This comment has been minimized.

Show comment
Hide comment
@marcospereira

marcospereira May 29, 2017

Member

It could be good to have benchmarks for a list of Employees too. Maybe with ~250 items on the list.

Also, why do we need a LinkedHashMap? According to JSON Spec "an object is an unordered set of name/value pairs."

Member

marcospereira commented May 29, 2017

It could be good to have benchmarks for a list of Employees too. Maybe with ~250 items on the list.

Also, why do we need a LinkedHashMap? According to JSON Spec "an object is an unordered set of name/value pairs."

@gmethvin

This comment has been minimized.

Show comment
Hide comment
@gmethvin

gmethvin May 30, 2017

Member

There was a discussion sometime in the past about changing this. The short story is that most JSON libraries for Scala treat JSON as an ordered list of fields, and our APIs in the past treated it as a list of fields, so we decided to use LinkedHashMap to keep the ordering.

You can pass any type of Map to JsObject, so if you want to use a more efficient type of map you can choose to do so.

Member

gmethvin commented May 30, 2017

There was a discussion sometime in the past about changing this. The short story is that most JSON libraries for Scala treat JSON as an ordered list of fields, and our APIs in the past treated it as a list of fields, so we decided to use LinkedHashMap to keep the ordering.

You can pass any type of Map to JsObject, so if you want to use a more efficient type of map you can choose to do so.

@gmethvin

This comment has been minimized.

Show comment
Hide comment
@gmethvin

gmethvin May 30, 2017

Member

@marcospereira I also added a test for lists. Seems like for a list of 100 elements it's a little more than 1/100 the speed, which makes sense. Here are more recent results:

[info] JsonMacros_01_SerializeSimpleCaseClass.toJson              thrpt   20  2123168.821 ±  51314.155  ops/s
[info] JsonMacros_01_SerializeSimpleCaseClass.toJsonManualWrites  thrpt   20  3079308.057 ± 107394.814  ops/s
[info] JsonMacros_02_SerializeList.toJson                         thrpt   20    15733.665 ±   1078.034  ops/s
[info] JsonMacros_02_SerializeList.toJsonManualWrites             thrpt   20    29426.515 ±   1299.593  ops/s
Member

gmethvin commented May 30, 2017

@marcospereira I also added a test for lists. Seems like for a list of 100 elements it's a little more than 1/100 the speed, which makes sense. Here are more recent results:

[info] JsonMacros_01_SerializeSimpleCaseClass.toJson              thrpt   20  2123168.821 ±  51314.155  ops/s
[info] JsonMacros_01_SerializeSimpleCaseClass.toJsonManualWrites  thrpt   20  3079308.057 ± 107394.814  ops/s
[info] JsonMacros_02_SerializeList.toJson                         thrpt   20    15733.665 ±   1078.034  ops/s
[info] JsonMacros_02_SerializeList.toJsonManualWrites             thrpt   20    29426.515 ±   1299.593  ops/s

@marcospereira marcospereira merged commit 651b57e into playframework:master Jun 3, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gmethvin gmethvin deleted the gmethvin:optimize branch Jun 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment