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

Support JRuby by removing fast_stack as a hard dependency #11

Merged
merged 1 commit into from Apr 17, 2015

Conversation

nilbus
Copy link
Contributor

@nilbus nilbus commented Apr 8, 2015

Since fast_stack uses a C extension, it is not compatible with JRuby. With this patch, fast_stack will be used when it is available, only after after using StackProf if that is available.

Tested manually on JRuby, and the specs still pass on MRI.

What do you think about this approach?

fast_stack will be used when it is available, only after
after using StackProf if that is available.
@SamSaffron
Copy link
Owner

In principal I am ok with this, but can you confirm you can get stack traces fast enough from JRuby to even make this project useful for JRuby ?

A screenshot perhaps?

@nilbus
Copy link
Contributor Author

nilbus commented Apr 17, 2015

Here's a screenshot, but how do you know what the sample rate is?

image 2015-04-08 at 2 32 06 pm

@SamSaffron
Copy link
Owner

That flamegraph does seem useful, you can get a rough measure of your
sample rate by figuring out how long it takes to execute the page without
flame graph and then dividing by the number of samples you got in the
graph.

On Fri, Apr 17, 2015 at 11:58 AM, Edward Anderson notifications@github.com
wrote:

Here's a screenshot, but how do you know what the sample rate is?

[image: image 2015-04-08 at 2 32 06 pm]
https://cloud.githubusercontent.com/assets/64751/7194788/ad001d6e-e483-11e4-8b72-2b825ad6b3a1.png


Reply to this email directly or view it on GitHub
#11 (comment).

SamSaffron added a commit that referenced this pull request Apr 17, 2015
Support JRuby by removing fast_stack as a hard dependency
@SamSaffron SamSaffron merged commit 3c15623 into SamSaffron:master Apr 17, 2015
@SamSaffron
Copy link
Owner

cool I merged this

@nilbus
Copy link
Contributor Author

nilbus commented Apr 17, 2015 via email

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.

None yet

2 participants