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

Huge performance boost by reducing operations with arrays #102

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

NikitaCartes
Copy link

Hi.

Script works about 2.2 times faster, If replace arrays ("a" and "t") in ecrypt/decrypt with int32 numbers.
Another 5% speed boost if delete convertToInt32 from this functions.

I performed some tests with 1000 runs of encryption 1MB data:

  1. Original JS file.
  2. After first commit.
  3. After second commit.

1
2
3

@NikitaCartes
Copy link
Author

NikitaCartes commented Jul 8, 2020

Last commit brings another 5% performance boost.

I did some tests with changes like #64, but i didn't notice any speed up.

4

@Antoine340
Copy link

Hello, this is a big improvement ! I tested it on production and it worked perfectly. Thank you very much !

@webdevelopland
Copy link

Works 1.8x faster for me now. Amazing!

@ricmoo
Copy link
Owner

ricmoo commented Apr 8, 2022

The docs need to be updated, as it now requires things to be Uint8Arrays, but can you compare against the typescript branch? It's available in npm with the beta tag.

@ricmoo
Copy link
Owner

ricmoo commented Apr 8, 2022

(the typescript branch is also designed to be backwards-compatibility breaking and allows for features in ES6 and beyond, while the older version is planned to continue targeting ES3 environments)

@ricmoo
Copy link
Owner

ricmoo commented Apr 8, 2022

(also note that segment size is now provided in bit-width; only byte-aligned segment sizes are currently supported, but that will change in the future)

@NikitaCartes
Copy link
Author

The docs need to be updated, as it now requires things to be Uint8Arrays, but can you compare against the typescript branch? It's available in npm with the beta tag.

But current version already work only with Uint8Arrays as stated in Readme?

Although I should use
createArray(roundKeyCount) instead of new Array(roundKeyCount)

@webdevelopland
Copy link

Hey, any news here?
The update is great, it makes the package much faster, so it makes no sense to use code before the update.
And everyone that wants the improvment has to hardcode the fork, instead of using npm.
The pull request is here for almost 2 years. How about to merge it already?

@webdevelopland
Copy link

@ricmoo @NikitaCartes
image

@ricmoo
Copy link
Owner

ricmoo commented Jan 11, 2024

Is that compared to the typescript branch? It’s been used in production for some time now and should be moved out of beta and made latest.

Thanks for the bump to look back into this.

@webdevelopland
Copy link

Current PR is based on main js branch. But it should be easy to convert it to typescript.
Once you release the TS update, I could help with this PR (unless @NikitaCartes don't wanna do it)

@NikitaCartes
Copy link
Author

I think I can do it when I will have some free time

@ricmoo
Copy link
Owner

ricmoo commented Feb 7, 2024

I’ll try getting the TypeScript branch merged this week. And put together some quick performance profiling.

@webdevelopland
Copy link

Thank you!

Repository owner deleted a comment from jalamari2018 Mar 19, 2024
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

4 participants