Skip to content

Speeding up decimal (de)serialization. #984

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
Dec 14, 2020

Conversation

stebet
Copy link
Contributor

@stebet stebet commented Dec 9, 2020

Proposed Changes

Serializing the decimal datatype allocates 40 bytes every time and deserializing it is sub-optimal. This optimization removes that allocation and speeds up (de)serializing decimal values

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause an observable behavior change in existing systems)
  • Documentation improvements (corrections, new content, etc)
  • Cosmetic change (whitespace, formatting, etc)

Checklist

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

Results

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.20270
Intel Core i7-10700 CPU 2.90GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.101
  [Host]        : .NET Core 3.1.10 (CoreCLR 4.700.20.51601, CoreFX 4.700.20.51901), X64 RyuJIT
  .NET 4.8      : .NET Framework 4.8 (4.8.4261.0), X64 RyuJIT
  .NET Core 3.1 : .NET Core 3.1.10 (CoreCLR 4.700.20.51601, CoreFX 4.700.20.51901), X64 RyuJIT

Before

Method Job Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated Code Size
DecimalRead .NET 4.8 21.939 ns 0.1721 ns 0.1526 ns - - - - 781 B
DecimalWrite .NET 4.8 29.016 ns 0.0880 ns 0.0735 ns 0.0063 - - 40 B 686 B
DecimalRead .NET Core 3.1 5.785 ns 0.0151 ns 0.0133 ns - - - - 449 B
DecimalWrite .NET Core 3.1 14.726 ns 0.2515 ns 0.2353 ns 0.0048 - - 40 B 300 B

After

Method Job Mean Error StdDev Gen 0 Gen 1 Gen 2 Allocated Code Size
DecimalRead .NET 4.8 12.192 ns 0.0406 ns 0.0360 ns - - - - 592 B
DecimalWrite .NET 4.8 13.177 ns 0.1229 ns 0.1089 ns - - - - 659 B
DecimalRead .NET Core 3.1 2.439 ns 0.0136 ns 0.0121 ns - - - - 227 B
DecimalWrite .NET Core 3.1 3.068 ns 0.0142 ns 0.0126 ns - - - - 284 B

@bollhals
Copy link
Contributor

bollhals commented Dec 9, 2020

I assume decimals aren't used as often, but the improvement is substantial nonetheless. 👍

@stebet
Copy link
Contributor Author

stebet commented Dec 9, 2020

I assume decimals aren't used as often, but the improvement is substantial nonetheless. 👍

I figured since it's a user-provided field, it might be used considerably in some cases (financial apps come to mind), so better to have most primitives as fast as possible.

@michaelklishin
Copy link
Contributor

@stebet can you please rebase this on top of the current master?

@stebet
Copy link
Contributor Author

stebet commented Dec 12, 2020

Will do. Might not happen until Monday though.

@michaelklishin
Copy link
Contributor

Sure, no rush :)

@stebet stebet force-pushed the decimalSerialization branch from 5af39fb to a935c43 Compare December 14, 2020 09:41
@stebet
Copy link
Contributor Author

stebet commented Dec 14, 2020

Sure, no rush :)

Done :)

@michaelklishin michaelklishin merged commit 830c747 into rabbitmq:master Dec 14, 2020
@michaelklishin
Copy link
Contributor

I assume this can be backported all the way to 6.x?

@stebet
Copy link
Contributor Author

stebet commented Dec 16, 2020

I assume this can be backported all the way to 6.x?

Yup :)

michaelklishin added a commit that referenced this pull request Dec 24, 2020
Speeding up decimal (de)serialization.

(cherry picked from commit 830c747)
@lukebakken lukebakken deleted the decimalSerialization branch February 14, 2022 17:36
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