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

Keep long bigint literals in hexadecimal format #4165

Open
KimlikDAO-bot opened this issue May 13, 2024 · 9 comments
Open

Keep long bigint literals in hexadecimal format #4165

KimlikDAO-bot opened this issue May 13, 2024 · 9 comments
Labels
help wanted internal-issue-created An internal Google issue has been created to track this GitHub issue P3 triage-done Has been reviewed by someone on triage rotation.

Comments

@KimlikDAO-bot
Copy link
Contributor

KimlikDAO-bot commented May 13, 2024

Long bigint literals written in hexadecimal format (such as 0x123456789ABDEFn) should be kept as is. Currently GCC expands them to decimal bigint literals, which increases the compiled size significantly (up to ~20%).

Example:

const P = 0x2fadbe2852044d028597455bc2abbd1bc873af205dfabb8a304600f3e09eeba8n;
console.log(P);

is transformed to

console.log(21565680844461314807147611702860246336805372493508489110556896454939225549736n)

Many crypto libraries (hashing, verifiable computation, etc) include long list of large constants. Keeping such constants in hexadecimal form makes a big difference.

Writing the literals as BigInt("0xabc") is a workaround but is not ideal due to increased compiled size and runtime cost.

This incurs up to 20% blow up in compiled code size.

@rishipal rishipal added triage-done Has been reviewed by someone on triage rotation. internal-issue-created An internal Google issue has been created to track this GitHub issue labels May 14, 2024
@KimlikDAO-bot
Copy link
Contributor Author

Actually, in the advanced more, both numbers and bigints can be emitted in hexadecimal if it makes the output shorter (literals larger than a certain threshold which can be calculated)

If true is transformed to !0, this optimization can also be considered, which may be equally effective.

@rishipal
Copy link
Contributor

This is a fair optimization request. We may not be able to prioritize this but we'd be happy to receive a PR of this change.

@rishipal rishipal added the P3 label May 15, 2024
@KimlikDAO-bot
Copy link
Contributor Author

Could you point me to the file where literals are emitted?

@brad4d
Copy link
Contributor

brad4d commented May 15, 2024

I think you'd modify the code approximately here.

KimlikDAO-bot added a commit to KimlikDAO-bot/closure-compiler that referenced this issue May 25, 2024
@KimlikDAO-bot
Copy link
Contributor Author

@brad4d Thank you. Where would the tests for this go?

@lauraharker
Copy link
Contributor

lauraharker commented May 28, 2024

@lauraharker
Copy link
Contributor

(Correction to my previous comment - I should have said https://github.com/google/closure-compiler/blob/master/test/com/google/javascript/jscomp/CodePrinterTest.java, sorry)

KimlikDAO-bot added a commit to KimlikDAO-bot/closure-compiler that referenced this issue May 28, 2024
KimlikDAO-bot added a commit to KimlikDAO-bot/closure-compiler that referenced this issue May 28, 2024
@KimlikDAO-bot
Copy link
Contributor Author

Thank you. Added some tests. Let me know if there other testing surfaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted internal-issue-created An internal Google issue has been created to track this GitHub issue P3 triage-done Has been reviewed by someone on triage rotation.
Projects
None yet
Development

No branches or pull requests

4 participants