Skip to content

Printing to ByteBuffer (bytes) #11

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

Merged
merged 1 commit into from
Jan 10, 2017

Conversation

vkostyukov
Copy link
Contributor

As discussed in circe/circe#536 and finagle/finch#676 this PR introduces jacksonPrintBytes that prints directly into ByteBuffer.

I run some benchmarks and the results are quite surprising. The throughput is never better when printing to bytes, but the allocation rate is significantly lower (2x for case classes). Not sure I can reasonably explain neither why allocations are nearly the same when printing int arrays nor why throughput isn't improved. Although, I think it's a reasonable first step in binary printing.

[info] Benchmark                                                              Mode  Cnt       Score       Error   Units
[info] PrintingBenchmark.printFoosC                                          thrpt   20    2894.216 ±   120.682   ops/s
[info] PrintingBenchmark.printFoosC:·gc.alloc.rate.norm                      thrpt   20  454564.587 ±  1799.636    B/op

[info] PrintingBenchmark.printFoosCJBytes                                    thrpt   20    3418.018 ±    50.857   ops/s
[info] PrintingBenchmark.printFoosCJBytes:·gc.alloc.rate.norm                thrpt   20  259067.149 ±    71.022    B/op

[info] PrintingBenchmark.printFoosCJString                                   thrpt   20    3547.072 ±    55.888   ops/s
[info] PrintingBenchmark.printFoosCJString:·gc.alloc.rate.norm               thrpt   20  464377.531 ±     2.872    B/op

[info] PrintingBenchmark.printIntsC                                          thrpt   20   18770.557 ±   360.538   ops/s
[info] PrintingBenchmark.printIntsC:·gc.alloc.rate.norm                      thrpt   20   74504.088 ±     0.173    B/op

[info] PrintingBenchmark.printIntsCJBytes                                    thrpt   20   35936.390 ±   962.170   ops/s
[info] PrintingBenchmark.printIntsCJBytes:·gc.alloc.rate.norm                thrpt   20   38496.047 ±     0.093    B/op

[info] PrintingBenchmark.printIntsCJString                                   thrpt   20   39662.014 ±   634.973   ops/s
[info] PrintingBenchmark.printIntsCJString:·gc.alloc.rate.norm               thrpt   20   42384.042 ±     0.082    B/op

@travisbrown
Copy link
Member

Thanks! I'm on the road for the next couple of hours but should be able to get these merged and 0.7.0-M2 published from the airport after that.

@codecov-io
Copy link

codecov-io commented Jan 9, 2017

Current coverage is 87.25% (diff: 100%)

Merging #11 into master will increase coverage by 0.79%

@@             master        #11   diff @@
==========================================
  Files            11         11          
  Lines            96        102     +6   
  Methods          87         93     +6   
  Messages          0          0          
  Branches          5          5          
==========================================
+ Hits             83         89     +6   
  Misses           13         13          
  Partials          0          0          

Powered by Codecov. Last update 1ba0880...56852cb

@vkostyukov
Copy link
Contributor Author

vkostyukov commented Jan 9, 2017

Thanks @travisbrown! I think it would be also nice to update README (w.r.t. benchmark results) at some point (when Circe's printing to bytes is also available).

@travisbrown
Copy link
Member

One quibble: what do you think about jacksonPrintByteBuffer, for consistency with the new prettyByteBuffer in circe-core, and to reserve the Bytes names for possible future Array[Byte] versions?

@vkostyukov
Copy link
Contributor Author

@travisbrown Agreed. Let me update this.

@travisbrown
Copy link
Member

Great, thanks!

@travisbrown travisbrown merged commit 00a6e91 into circe:master Jan 10, 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.

3 participants