-
Notifications
You must be signed in to change notification settings - Fork 11
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
Split object_space_dump into smaller messages #77
Conversation
@emilsoman can you document Message format after this change? Potentially in a new WIKI page so as not to remove existing one. |
@gnufied , the only change is a new key that's added to the payload of each object space dump message called "snapshot_no" , I've added this to the wiki already |
@emilsoman can you update this PR so as it is mergeable? |
@gnufied , done. |
// messages. This macro defines the number | ||
// of object data that should be packed | ||
// as the payload of one message. | ||
#define MAX_OBJECT_DUMPS_IN_MESSAGE 20 |
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.
20 is very small number emil and I think for larger projects we will see drop in messages, because queue will be too full.
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 think this number has to be way higher, but it has to be calibrated. But something like 1000 should not be too bad of a limit IMO.
@emilsoman can we update this PR with latest master?! |
This version of protocol is not compatible with previous version.
Okay, so this looks mostly good. One problem though is - this specifically implements splitting of heap dumps which is fine. But I think, we will require similar splitting of larger cpu profiling dumps, so it makes sense to make it bit generic. I think it makes sense to add a new field called What I am really proposing is simply renaming |
On seconds thoughts, adding So really, my only request is to just rename the field itself. :-) |
Split object_space_dump into smaller messages
Closes #52
Merge after #76 is merged.
Stuff that changed :