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

Add benchmark reporting to PRs and commits #455

Merged
merged 11 commits into from
Aug 18, 2024
Merged

Conversation

XantreDev
Copy link
Contributor

@XantreDev XantreDev commented Aug 11, 2024

Closes #253

@XantreDev XantreDev changed the title implemented benchmarks Implemented benchmarks Aug 11, 2024
@github-actions github-actions bot added the context-v2 Related to tailwind-merge v2 label Aug 11, 2024
Copy link

codspeed-hq bot commented Aug 11, 2024

CodSpeed Performance Report

Congrats! CodSpeed is installed 🎉

🆕 7 new benchmarks were detected.

You will start to see performance impacts in the reports once the benchmarks are run from your default branch.

Detected benchmarks

  • collection(cached) (14.1 ms)
  • heavy(cached) (401.9 µs)
  • simple(cached) (395.1 µs)
  • init (4.9 ms)
  • collection (199.8 ms)
  • heavy (316.8 µs)
  • simple (85.4 µs)

@XantreDev
Copy link
Contributor Author

XantreDev commented Aug 12, 2024

It's working, but look like - we cannot see performance details. I will try to use vitest integration. Maybe it will help
Reported the bug

@XantreDev XantreDev marked this pull request as ready for review August 13, 2024 08:35
@XantreDev XantreDev marked this pull request as draft August 13, 2024 08:38
@XantreDev
Copy link
Contributor Author

I feel that I need to add benchmarks without cache. Because now we are benchmarking LRUCache performance. It's ok, but I do not think that's what we want

@XantreDev
Copy link
Contributor Author

Wow. Profiling info looks nice
image

@XantreDev XantreDev marked this pull request as ready for review August 14, 2024 22:45
Copy link
Owner

@dcastil dcastil left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the great work! I added some comments about some minor stuff and a bigger one regarding the caching.

Could you address those?

.github/workflows/codeql-analysis.yml Outdated Show resolved Hide resolved
.github/workflows/codspeed.yml Outdated Show resolved Hide resolved
.github/workflows/codspeed.yml Outdated Show resolved Hide resolved
benchmarks/twMerge.bench.ts Outdated Show resolved Hide resolved
benchmarks/twMerge.bench.ts Outdated Show resolved Hide resolved
@dcastil dcastil added the documentation Improvements or additions to documentation label Aug 17, 2024
@XantreDev
Copy link
Contributor Author

it works locally, but do not work with plugin xD

@XantreDev
Copy link
Contributor Author

XantreDev commented Aug 17, 2024

It seems like of ci codespeed calls function only once to measure performance
But I am no sure

Copy link
Owner

@dcastil dcastil left a comment

Choose a reason for hiding this comment

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

Looks good!

It seems like of ci codespeed calls function only once to measure performance
But I am no sure

We'll figure it out as we go. After a few other PRs it will be easier to tell whether something is off. I'll also play around a bit with it.

@dcastil dcastil merged commit 34753fe into dcastil:main Aug 18, 2024
4 of 5 checks passed
@dcastil dcastil changed the title Implemented benchmarks Add benchmark reporting to PRs Aug 18, 2024
@dcastil dcastil changed the title Add benchmark reporting to PRs Add benchmark reporting to PRs and commits Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context-v2 Related to tailwind-merge v2 documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add performance test
2 participants