Skip to content

Tweak size #63

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

Tweak size #63

wants to merge 7 commits into from

Conversation

jeetiss
Copy link
Contributor

@jeetiss jeetiss commented Jun 8, 2023 •
edited
Loading

I applied some optimizations to reduce bundle size:

  • sideEffects: false to improve dead code elimination
  • removed default export in _u64
  • migrated _assert and _u64 module to wildcard imports to enable tree-shaking
  • added /*#__PURE__*/ annotation to all top-level invocations

sizes (with all dependencies, minified and gzipped):

blake2b from blake2b
Size:         3 kB      →   2.71 kB

blake2s from blake2s
Size:         2.75 kB   →   2.31 kB

blake3 from blake3
Size:         4.22 kB   →   3.19 kB

hmac from hmac
Size:         1.03 kB   →   891 B

hkdf from hkdf
Size:         1.25 kB   →   1.11 kB

pbkdf2 from pbkdf2
Size:         1.47 kB   →   1.33 kB

pbkdf2Async from pbkdf2
Size:         1.54 kB   →   1.41 kB

ripemd160 from ripemd160
Size:         2.15 kB   →   1.98 kB

scrypt from scrypt
Size:         4.32 kB   →   4.13 kB

scryptAsync from scrypt
Size:         4.39 kB   →   4.2 kB

sha256 from sha256
Size:         2.45 kB   →   2.18 kB

sha512 from sha512
Size:         4.09 kB   →   3.44 kB

sha1 from sha1
Size:         1.82 kB   →   1.65 kB

sha3_224 from sha3
Size:         2.06 kB   →   1.79 kB

sha3_256 from sha3
Size:         2.06 kB   →   1.79 kB

sha3_384 from sha3
Size:         2.06 kB   →   1.79 kB

sha3_512 from sha3
Size:         2.06 kB   →   1.79 kB

keccakP from sha3
Size:         2.06 kB   →   705 B

keccak_256 from sha3
Size:         2.06 kB   →   1.79 kB

keccak_384 from sha3
Size:         2.06 kB   →   1.79 kB

keccak_512 from sha3
Size:         2.06 kB   →   1.79 kB

cshake128 from sha3-addons
Size:         2.32 kB   →   2.04 kB

cshake256 from sha3-addons
Size:         2.32 kB   →   2.04 kB

kmac128 from sha3-addons
Size:         2.5 kB    →   2.2 kB

kmac256 from sha3-addons
Size:         2.5 kB    →   2.19 kB

k12 from sha3-addons
Size:         2.51 kB   →   2.24 kB

m14 from sha3-addons
Size:         2.51 kB   →   2.24 kB

argon2id from argon2
Size:         4.9 kB    →   4.6 kB

eskdf from eskdf
Size:         5.43 kB   →    5.28 kB

i used #62 to test changes

jeetiss
jeetiss commented Jun 8, 2023
View reviewed changes
jeetiss
jeetiss commented Jun 8, 2023
View reviewed changes
@paulmillr
Copy link
Owner

run big-endian check in every entry and mark it as sideEffect

why? does sideEffects: false in package.json help us in any way? i've thought only PURE annotations do.

@jeetiss
Copy link
Contributor Author

jeetiss commented Jun 10, 2023

  • sideEffects: false allows to webpack skip unused imports earlier and not traverse over all module tree
  • sideEffects: false decrease the bundle size for all entries ~100B-300B
  • it also breaks the big-endian check because webpack will remove it
  • I moved the check to a separate file and marked it as a side effect so webpack would enable that greedy tree-shaking and would keep the big-endian check always in bundle

made a little demo with stackblitz to test that behavior

@jeetiss
Copy link
Contributor Author

jeetiss commented Jun 22, 2023

@paulmillr Hello,
Are you interested in those changes? I wanna create some PR for the curves package.

@paulmillr
Copy link
Owner

@jeetiss thanks for the pull request. It's good. Yes, I am interested.

I am concerned with moving-out big-endian check into the separate file.

It's always used. Can't we just keep it in utils? I assume sideEffects: false remove it, but what else do they remove?

@paulmillr
Copy link
Owner

Please don't make curves PR until we merge the current one, don't want you to do work that won't be merged.

@jeetiss
Copy link
Contributor Author

jeetiss commented Jun 25, 2023

I think it should be better to split this PR into a couple:

  • one with pure notations
  • second with migration from default exports to wildcard imports
  • and third with sideEffects: false

The first two are not so controversial and we can discuss options for sideEffects after

This was referenced Jun 25, 2023
Merged
Merged
Closed
@jeetiss jeetiss closed this Jun 25, 2023
@jeetiss jeetiss deleted the tweak-size branch June 25, 2023 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulmillr paulmillr paulmillr left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jeetiss @paulmillr