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

Bring profiler to 2020 #15691

Merged
merged 5 commits into from
Jun 1, 2020
Merged

Bring profiler to 2020 #15691

merged 5 commits into from
Jun 1, 2020

Conversation

rafatower
Copy link
Contributor

A couple of light touches to the best tool under my belt to profile slow ruby code.

Amazed it almost worked out of the box 5 years later.

The biggest change is returning the profile trace as the request payload, instead of making you look it up in rui servers in production.

See this wiki page (I know, update pending): https://github.com/CartoDB/cartodb-management/wiki/Profiling-rails

cc/ @CartoDB/backend-team FWIW

have a nice 🐸 time!

@rafatower rafatower requested a review from Algunenano May 29, 2020 16:54
Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

❤️

@Algunenano
Copy link
Contributor

LGTM. I'm building this locally to test, but I understand that if I do a request with the profile_request=true and the feature flag on, the browser should download the callgrind file automagically right?

Pinging @alasarr since we was also thinking on using this and it might be really useful now.

Copy link
Contributor

@Algunenano Algunenano left a comment

Choose a reason for hiding this comment

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

Tested and it looks good.

There is an issue though. Altough it's not beeing used, it still requires the directory to exist in the server, and if it doesn't it returns a 500 error.

Since the callgrind traces are sent in the body of the request,
there's no need to create a directory to save callgrind traces in the
filesystem.

Remove code that created that directory and failed if the parent did
not exist, producing a 500 error.
@rafatower
Copy link
Contributor Author

it still requires the directory to exist in the server, and if it doesn't it returns a 500 error.

good catch, fixing it straight away!

@rafatower rafatower merged commit 1ace056 into master Jun 1, 2020
@rafatower rafatower deleted the bring-profiler-to-2020 branch June 1, 2020 09:22
@rafatower
Copy link
Contributor Author

PS: wiki page updated as well.

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