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 Decimal conversion performance using _mantissa #124

Merged
merged 4 commits into from
Nov 11, 2024

Conversation

mredig
Copy link
Contributor

@mredig mredig commented Nov 8, 2024

There seems to be a public-ish field called _mantissa in Decimal which should be faster than using the string representation here. Maybe we should use this and keep the above arround maybe commented out just in case _mantissa goes away one day.

Originally posted by @tgymnich in #123 (comment)

According to my tests, it looks like using the _mantissa approach is roughly 1/3 faster.

Starting 'Decimal String Init' with 999999 iterations.
Test 'Decimal String Init' with 999999 iterations took 14.572233080863953 seconds.
Finished 'Decimal String Init' with 999999 iterations.


Starting 'Decimal Mantissa Init' with 999999 iterations.
Test 'Decimal Mantissa Init' with 999999 iterations took 10.257248044013977 seconds.
Finished 'Decimal Mantissa Init' with 999999 iterations.


╔═══════════════════════╤══════════════╤══════════════╤══════════════╤══════════════╤══════════════╗
║ Label                 │ Avg          │ Max          │ Min          │ Delta        │ Median       ║
╟───────────────────────┼──────────────┼──────────────┼──────────────┼──────────────┼──────────────╢
║ Decimal Mantissa Init │ 0.0000102246 │ 0.0002379417 │ 0.0000089407 │ 0.0002290010 │ 0.0000110865 ║
╟───────────────────────┼──────────────┼──────────────┼──────────────┼──────────────┼──────────────╢
║ Decimal String Init   │ 0.0000144051 │ 0.0035669804 │ 0.0000129938 │ 0.0035539865 │ 0.0000140667 ║
╚═══════════════════════╧══════════════╧══════════════╧══════════════╧══════════════╧══════════════╝

I'll have another push in a bit to address the second suggestion.

@mredig
Copy link
Contributor Author

mredig commented Nov 8, 2024

Oh well look at that - linux doesn't like optimizations! Apparently "public-ish" was more on point than I thought.

@mredig
Copy link
Contributor Author

mredig commented Nov 8, 2024

Works, but setting the remaining bytes (0..<i) to zero after the loop below could be more efficient.

Originally posted by @tgymnich in #123 (comment)

@mredig
Copy link
Contributor Author

mredig commented Nov 8, 2024

Bonus - I realized I needed Decimal(_ value: Big(U)Int and added that. I might be able to figure out a mantissa init instead of string based, but brain now dead. 🧟

Copy link
Collaborator

@tgymnich tgymnich left a comment

Choose a reason for hiding this comment

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

👍

@tgymnich tgymnich merged commit 114343a into attaswift:master Nov 11, 2024
4 checks passed
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.

2 participants