Skip to content

Use __index__ if needed to pack integer values in struct#1383

Merged
slozier merged 1 commit intoIronLanguages:masterfrom
BCSharp:struct__index__
Mar 28, 2022
Merged

Use __index__ if needed to pack integer values in struct#1383
slozier merged 1 commit intoIronLanguages:masterfrom
BCSharp:struct__index__

Conversation

@BCSharp
Copy link
Copy Markdown
Member

@BCSharp BCSharp commented Mar 28, 2022

Resolves #1381.

Copy link
Copy Markdown
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

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

Looks good to me. Interesting that CPython has two different error messages for some of these.

Random thought, I wonder if trying to cast the BigInteger and catching the failure would be more performant than checking the range.

@slozier slozier merged commit f6fbf9b into IronLanguages:master Mar 28, 2022
@BCSharp
Copy link
Copy Markdown
Member Author

BCSharp commented Mar 28, 2022

Intuitively, a direct cast should be faster for cases without overflow, but I was curious by how much, so I did some measurements.

The results show that going through AstInt32 adds ca. 550% to the execution of the cast (i.e., the whole operation is 6.5 times slower). About half of this overhead comes from the two BigInteger comparisons, and the other half from the static method call (and whatever else that method does).

That said, those things are extremely fast (and hard to measure accurately). Converting by calling AstInt32 on my machine is less than 8 nanoseconds. So in the end, it is a matter of judgment if shaving off a few nanoseconds is worth increased code verbosity.

@BCSharp BCSharp deleted the struct__index__ branch March 28, 2022 21:23
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.

struct.pack does not respect __index__ operator

2 participants