Skip to content

Conversation

@iboB
Copy link
Contributor

@iboB iboB commented Feb 4, 2021

In case the base is less than 10 it is correct to calculate the digit by adding it to '0' instead of looking it up in the table. This is a small optimization

@iboB iboB requested a review from a team as a code owner February 4, 2021 12:37
@StephanTLavavej StephanTLavavej added the performance Must go faster label Feb 4, 2021
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks! I'm not sure whether these bases are used in practice, but this change is simple and clearly correct (we also have extensive test coverage for all bases).

(Further optimizing these unusual bases probably wouldn't be worth additional code complexity - all code has a maintenance cost - but the cost here is essentially zero.)

(It's unfortunate that the Standard doesn't provide the base as a template argument, which would allow us to use if constexpr in the default case.)

@StephanTLavavej StephanTLavavej self-assigned this Feb 10, 2021
@StephanTLavavej StephanTLavavej merged commit 572d87e into microsoft:main Feb 12, 2021
@StephanTLavavej
Copy link
Member

Thanks for optimizing my favorite header, and congratulations on your first microsoft/STL commit! 🎉 🚀 😸

@iboB
Copy link
Contributor Author

iboB commented Feb 13, 2021

Thanks!
BTW I came across this as I was printing integers in base 3. Not that the perf matters that much, but I just told to myself 'Well, why not?' 😄

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

Labels

performance Must go faster

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants