-
Notifications
You must be signed in to change notification settings - Fork 360
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 CPU profiling and heap snapshot information; add IntelliJ files to .gitignore #114
base: master
Are you sure you want to change the base?
Conversation
@@ -8,3 +8,9 @@ build/ | |||
*.dll | |||
*.dylib | |||
hs_err*.log | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use IntelliJ rather than Eclipse, and there are a different set of local user-specific project files that should be ignored by Git
This is GREAT. Thanks! Can you split this into two separate commits, one for the changes to .gitignore and one for the profiling. Adding the IntelliJ files to .gitignore makes sense. The profile change I would like to look at closer and see if we can add some test cases. |
Sure. I can do that. It might make sense to also include the build script change along with the .gitignore changes. The profiling changes are definitely more complex. I wasn't sure about test cases as some of the profiling (CPU and memory) won't be deterministic and my primary method of testing and validation was loading the files in Google Chrome. I could certainly add some tests for some of the util methods and JsonSerializer if desired. |
Yeah, feel free to include the change to the build script. Thanks! On Wed, Dec 2, 2015 at 12:49 PM, David Riggleman [email protected]
R. Ian Bull | EclipseSource Victoria | +1 250 477 7484 |
d675e71
to
7e10f2c
Compare
Thanks so much! I've pushed some of these changes through, and I've managed to get the CPU & Heap profile stuff to work with 4.x. I want to assemble a few unit tests for these before I commit them. I will complete this review and push this in. Thanks again @DavidRigglemanININ! |
f18cca7
to
daf068c
Compare
cd57118
to
4b8e529
Compare
These changes allow for profiling (both CPU and memory) server side code using J2V8 and produces a file that can be loaded into the Google Chrome developer tools for further analysis and visual display.
Unless the newly added methods are invoked, these changes have no impact on existing code. I wasn't exactly sure what your requirements are for pull requests, so please let me know if I'm missing anything you expected was there. Also, I'm a little rusty on my C++ skills, so feel free to mention if there might be better ways to do something.