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

Optimize Actions.(dec|hex|oct|bin)int() #1021

Merged
merged 2 commits into from Feb 24, 2017

Conversation

MasterDuke17
Copy link
Contributor

Create a string_to_int that uses nqp::radix instead of nqp::radix_I.
Then call that from Actions.(dec|hex|oct|bin)int() instead of
string_to_bigint when the number of chars in the string to be converted
is small enough that it couldn't cause overflow.

Passes make m-spectest.

If there was a way to tell whether we're on a 32 or 64 bit system in
Actions.nqp the number of chars at which we switch to nqp::radix_I
could be increased.

Create a string_to_int that uses nqp::radix instead of nqp::radix_I.
Then call that from Actions.(dec|hex|oct|bin)int() instead of
string_to_bigint when the number of chars in the string to be converted
is small enough that it couldn't cause overflow.
@samcv
Copy link
Member

samcv commented Feb 21, 2017

Did you see any difference in speed?

@lizmat
Copy link
Contributor

lizmat commented Feb 21, 2017

This is the code in src/core/Rakudo/Internals.pm:

my constant $?BITS = do {
my int $a = 0x1ffffffff;
nqp::iseq_i($a,8589934591) ?? 64 !! 32
}

I think something like that can work in NQP as well :-)

@MasterDuke17
Copy link
Contributor Author

@lizmat thanks, I"ll add that.

@samcv in my testing, the inclusive time for decint (which just calls string_to_(big)?int) went down. Its exclusive time increased, but by less than the combination of string_to_int+string_to_bigint exclusive time decreased, so there was an overall savings. decint ended up being something like 20% faster by inclusive time, but it's so fast to begin with that the overall savings aren't very noticeable.

@MasterDuke17
Copy link
Contributor Author

@samcv, @lizmat added a commit that adjusts when to switch to string_to_bigint based on whether the system is 32 or 64 bit.

@lizmat lizmat merged commit d41b68e into rakudo:nom Feb 24, 2017
@MasterDuke17 MasterDuke17 deleted the optimize_string_to_bigint branch February 24, 2017 13:32
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.

None yet

3 participants