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

Improve runtime performance #333

Closed
sindresorhus opened this issue Mar 18, 2019 · 4 comments · Fixed by #337
Closed

Improve runtime performance #333

sindresorhus opened this issue Mar 18, 2019 · 4 comments · Fixed by #337
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Milestone

Comments

@sindresorhus
Copy link
Member

sindresorhus commented Mar 18, 2019

Issuehunt badges

Chalk is already fast enough for most usage. Benchmarks in readme's like this one are silly because no one will run Chalk in a constant loop, and second they're also false, because those packages do less things than Chalk, so of course they're faster. However, that doesn't mean we shouldn't try to be as fast as possible.

Some things we could consider:

  • Adding fast-paths for common use-cases. I think most people just do chalk.red(), chalk.bold.green(), etc (Meaning only 16 colors). We could add some fast-paths for those cases.
  • Caching whenever possible.
  • Experiment with using Proxy. This would reduce our method usage, but might add different kinds of overhead. Needs to me measured.
  • Other things. Open to ideas.

Solving this tasks should also include measuring performance in DevTools and improving the benchmark file in this repo.

Chalk targets Node.js 8, but the optimizations should target Node.js 10.

This is a difficult issue and if you want to tackle it you're expected to have a deep level of understanding of JS and be familiar with measuring performance in DevTools.


IssueHunt Summary

stroncium stroncium has been rewarded.

Backers (Total: $100.00)

Submitted pull Requests


Tips


IssueHunt has been backed by the following sponsors. Become a sponsor

@Qix-
Copy link
Member

Qix- commented Mar 18, 2019

Meaning only 16-bit colors

4-bit*, 16 total.

We could add some fast-paths for those cases.

Such as?

Caching whenever possible.

You mean memoizing?

Experiment with using Proxy. This would reduce our method usage, but might add different kinds of overhead. Needs to me measured.

Proxy has a really bad history of being slow (I remember really wanting them when they came out for some ORM-type of code that just wasn't performant).

But of course, let's indeed measure it.

@sindresorhus
Copy link
Member Author

We could add some fast-paths for those cases.

Such as?

That should be based on performance measurement, but one idea is to not load color-convert in those cases.

You mean memoizing?

Yes

Proxy has a really bad history of being slow (I remember really wanting them when they came out for some ORM-type of code that just wasn't performant).

It has improved a lot in recent V8 versions.

https://v8.dev/blog/optimizing-proxies

@IssueHuntBot
Copy link

@IssueHunt has funded $100.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 10, 2019
@issuehunt-oss
Copy link

issuehunt-oss bot commented Jul 12, 2019

@sindresorhus has rewarded $90.00 to @stroncium. See it on IssueHunt

  • 💰 Total deposit: $100.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $10.00

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants