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

chore: refactor by removing a couple variables #20

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

styfle
Copy link
Contributor

@styfle styfle commented Jul 18, 2023

@styfle
Copy link
Contributor Author

styfle commented Jul 18, 2023

@H4ad Do you have the systems you benchmarked from #19 to try this out on to make sure I didn't slow anything down?

@H4ad
Copy link
Contributor

H4ad commented Jul 18, 2023

@styfle No problem, same overall performance, could be a bit slower because of .toLowerCase, but I don't think it's enough to slow down in this case.

Copy link
Owner

@lovell lovell left a comment

Choose a reason for hiding this comment

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

Given the following:

  • we're thinking about micro-optimisations
  • we only need to check for the string literal "GLIBC" in all capitals (or the string literal "musl" in all lower case)
  • the length of /usr/bin/ldd file on the latest Ubuntu is 5390 bytes

how about these changes?

lib/detect-libc.js Outdated Show resolved Hide resolved
lib/detect-libc.js Outdated Show resolved Hide resolved
lib/detect-libc.js Outdated Show resolved Hide resolved
@lovell lovell merged commit 8cf409b into lovell:main Jul 19, 2023
38 checks passed
@styfle styfle deleted the chore-refactor-lowercase-uppercase branch July 19, 2023 20:19
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.

3 participants