Open GoogleCodeExporter opened 9 years ago
I went ahead and implemented this. It does not break any unit tests and it
seems to work fine on my system (RHEL 6.6).
Patch is attached. Let me know what I can do to make it suitable for merging.
Original comment by lopre...@gmail.com
on 8 May 2015 at 10:19
Attachments:
Looks good. Thanks.
My only little concern is that this patch changes semantic of
CPUPROFILE_PER_THREAD_TIMERS environment variable. I.e. before this patch
somebody could be setting it to 1 or 0. After this patch it'll be interpreted
as signal number.
I think it would be even better to just add separate environment variable for
new behavior (or new variable for signal number).
Original comment by alkondratenko
on 9 May 2015 at 10:24
[deleted comment]
Given that this feature was never even documented (except in the release
announcement on the home page), and it never even had a unit test, I would
argue it is simpler just to change the behavior and document the change...
Also, note that setting the environment variable to the empty string still
works the same as before.
But you are the maintainer, so your opinion wins :-). If you can make a
detailed suggestion, I will implement it and send you a patch... Note that a
separate environment variable for the new behavior would be mutually exclusive
with CPUPROFILE_PER_THREAD_TIMERS. A new variable for the signal number would
require (or imply) CPUPROFILE_PER_THREAD_TIMERS. It is not clear to me what
approach is cleanest.
Let me know your preference. Thanks.
Original comment by lopre...@gmail.com
on 10 May 2015 at 12:20
Something like this:
* if only CPUPROFILE_PER_THREAD_TIMERS is given, default signal is used
* if both CPUPROFILE_PER_THREAD_TIMERS and new flag is given, new behavior
happens
* if just new flag is given, new behavior happens
* if none of flags are given, traditional behavior happens (i.e. setitimer)
I think this would make most sense. Let me know if you disagree or if it's not
clear.
And huge thanks for taking care of this.
Original comment by alkondratenko
on 10 May 2015 at 1:35
Updated patch is attached. This adds a "CPUPROFILE_TIMER_SIGNAL" environment
variable with the semantics you describe. Note that this environment variable
also enables "per-thread" timers as a side-effect, so it pretty much subsumes
the other option.
Original comment by lopre...@gmail.com
on 10 May 2015 at 9:52
Attachments:
Thanks. Looks good. I'll take another look next Saturday and merge.
Thanks again.
Original comment by alkondratenko
on 17 May 2015 at 6:55
Original issue reported on code.google.com by
lopre...@gmail.com
on 8 May 2015 at 6:07