Skip to content

feat(encoding): improve hex conversion performance #1712

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

reallesee
Copy link

Replace string concatenation in toHex() with Array.from().map().join() pattern for better performance with large arrays. This approach avoids creating intermediate string objects during concatenation, resulting in better memory usage and execution speed, especially when processing large byte arrays

Copy link
Member

@webmaster128 webmaster128 left a comment

Choose a reason for hiding this comment

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

Sounds good. I just wonder if we can avoid creating the Array<number> in the first line which would be quite some overhead if not optimized away.

Would it be an option to do something like data.values().map((byte) => ...)? Do you have tests to measure the performance?

@reallesee
Copy link
Author

You're right about the Array.from() overhead - thank you.

I could use the spread syntax instead:

export function toHex(data: Uint8Array): string {
  return [...data].map((byte) => byte.toString(16).padStart(2, "0")).join("");
}

Or for maximum efficiency, reduce without intermediate arrays:

export function toHex(data: Uint8Array): string {
  return Array.prototype.reduce.call(
    data, 
    (str, byte) => str + byte.toString(16).padStart(2, "0"), 
    ""
  );
}

I don't have benchmarks yet, but I can run some tests if you'd like to see which approach performs best with different data sizes. What do you think?

@webmaster128
Copy link
Member

Getting some benchmarks in place would be very helpful to ensure we don't accidentally slow things down.

[...data] also creates a Array<number>, doesn't it? The reduce implementation creates all those intermediate strings we had before in the str accumulator, so I guess this is not better than the current implementation.

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.

2 participants