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 rack-mini-profiler gem #4956

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

Conversation

coalest
Copy link
Collaborator

@coalest coalest commented Jan 28, 2025

Description

We have some pages that are known to be slow. Adding this gem might make slow performance somewhat more obvious for contributors and potentially easier to debug.

For example, in one issue I remember a contributor asking for a flamegraph to triage a slow request. Adding this gem lowers the bar for contributors to work on performance-related issues.

I also thing it might help when testing PRs or features, to see if a PR is adding a N+1 or something that will cause a performance degradation.

Documentation on the gem can be found here: https://github.com/MiniProfiler/rack-mini-profiler

If someone find the badge annoying, it can be hidden by pressing alt+p.

Type of change

  • Internal (gem only added to development env)

How Has This Been Tested?

I don't think testing is necessary

Screenshots

Some example screens:
Screenshot from 2025-01-28 10-58-14
Screenshot from 2025-01-28 11-15-35
image

@dorner
Copy link
Collaborator

dorner commented Jan 29, 2025

@coalest do we still need bullet if we have this?

@coalest
Copy link
Collaborator Author

coalest commented Jan 29, 2025

I still find it useful for the following reasons:

1.It provides a warning when eager loading an association that then isn't used/needed (with rack mini profiler, you would have to go through the SQL queries and check manually for the extra query).
2. And it's still helpful for catching N+1's because with the seed data, we may only have a few records on an index page. So an N+1 might cause 3 queries instead of 1, which isn't very noticeable just by looking at the SQL query count.

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