Re: [PATCH v5 00/11] add performance tracing facility
On Wed, Jun 25, 2014 at 9:49 PM, Karsten Blees wrote: > Am 25.06.2014 16:28, schrieb Duy Nguyen: >> On Wed, Jun 11, 2014 at 2:55 PM, Karsten Blees >> wrote: >>> Here's v5 of the performance tracing patch series, now including a bunch of >>> cleanups and adding timestamp, file and line to all trace output. >>> >>> I'm particularly interested in feedback for the output format. As file >>> names have different lengths, printing file:line as prefix results in >>> unaligned output: >>> >>> > GIT_TRACE=1 git stash list >>> 00:12:10.544266 git.c:512 trace: exec: 'git-stash' 'list' >>> 00:12:10.544266 run-command.c:337 trace: run_command: 'git-stash' 'list' >>> 00:12:10.649779 git.c:312 trace: built-in: git 'rev-parse' '--git-dir' >> >> Can I have an (build-time) option to show : instead of >> :? I know it's not supported by all compilers, which may >> make support a bit cumbersome.. >> > > Is this really useful? : is unique, but : is not. > E.g. in case of "hash_name:47" you'd have to guess if its the one in attr.c > or name-hash.c... It depends on your view. If I'm tracing a certain operation, function names let me know roughly what's going on without looking at the code because all (or many of) the names and their purposes are already in my mind. But for publishing the traces, then I agree : is better. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/11] add performance tracing facility
Am 25.06.2014 16:28, schrieb Duy Nguyen: > On Wed, Jun 11, 2014 at 2:55 PM, Karsten Blees > wrote: >> Here's v5 of the performance tracing patch series, now including a bunch of >> cleanups and adding timestamp, file and line to all trace output. >> >> I'm particularly interested in feedback for the output format. As file names >> have different lengths, printing file:line as prefix results in unaligned >> output: >> >> > GIT_TRACE=1 git stash list >> 00:12:10.544266 git.c:512 trace: exec: 'git-stash' 'list' >> 00:12:10.544266 run-command.c:337 trace: run_command: 'git-stash' 'list' >> 00:12:10.649779 git.c:312 trace: built-in: git 'rev-parse' '--git-dir' > > Can I have an (build-time) option to show : instead of > :? I know it's not supported by all compilers, which may > make support a bit cumbersome.. > Is this really useful? : is unique, but : is not. E.g. in case of "hash_name:47" you'd have to guess if its the one in attr.c or name-hash.c... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/11] add performance tracing facility
On Wed, Jun 11, 2014 at 2:55 PM, Karsten Blees wrote: > Here's v5 of the performance tracing patch series, now including a bunch of > cleanups and adding timestamp, file and line to all trace output. > > I'm particularly interested in feedback for the output format. As file names > have different lengths, printing file:line as prefix results in unaligned > output: > > > GIT_TRACE=1 git stash list > 00:12:10.544266 git.c:512 trace: exec: 'git-stash' 'list' > 00:12:10.544266 run-command.c:337 trace: run_command: 'git-stash' 'list' > 00:12:10.649779 git.c:312 trace: built-in: git 'rev-parse' '--git-dir' Can I have an (build-time) option to show : instead of :? I know it's not supported by all compilers, which may make support a bit cumbersome.. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/11] add performance tracing facility
Am 12.06.2014 20:30, schrieb Junio C Hamano:
> Karsten Blees writes:
>
>> Here's v5 of the performance tracing patch series, now including a bunch of
>> cleanups and adding timestamp, file and line to all trace output.
>>
>> I'm particularly interested in feedback for the output format. As file names
>> have different lengths, printing file:line as prefix results in unaligned
>> output:
>>
>> > GIT_TRACE=1 git stash list
>> 00:12:10.544266 git.c:512 trace: exec: 'git-stash' 'list'
>> 00:12:10.544266 run-command.c:337 trace: run_command: 'git-stash' 'list'
>> 00:12:10.649779 git.c:312 trace: built-in: git 'rev-parse' '--git-dir'
>>
>> We could add separators to make it easier to parse, e.g.:
>>
>> > GIT_TRACE=1 git stash list
>> [00:12:10.544266 git.c:512] trace: exec: 'git-stash' 'list'
>> [00:12:10.544266 run-command.c:337] trace: run_command: 'git-stash' 'list'
>> [00:12:10.649779 git.c:312] trace: built-in: git 'rev-parse' '--git-dir'
>
> This is easier to parse if " " and ":" are found in the names of
> _our_ source files and "]" isn't, but is that really the case?
>
By "parsing" I actually meant the HumanEye (tm) parser, not lex/yacc and
friends ("[]" just make nice recognizable separators).
However, I think it shouldn't be too complicated to properly align the output,
at least for the majority of 'short' file names in the git code base. E.g.:
00:12:10.544266 git.c:512trace: exec: 'git-stash' 'list'
00:12:10.544266 run-command.c:337trace: run_command: 'git-stash' 'list'
00:12:10.649779 git.c:312trace: built-in: git 'rev-parse'
'--git-dir'
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/11] add performance tracing facility
Karsten Blees writes: > Here's v5 of the performance tracing patch series, now including a bunch of > cleanups and adding timestamp, file and line to all trace output. > > I'm particularly interested in feedback for the output format. As file names > have different lengths, printing file:line as prefix results in unaligned > output: > > > GIT_TRACE=1 git stash list > 00:12:10.544266 git.c:512 trace: exec: 'git-stash' 'list' > 00:12:10.544266 run-command.c:337 trace: run_command: 'git-stash' 'list' > 00:12:10.649779 git.c:312 trace: built-in: git 'rev-parse' '--git-dir' > > We could add separators to make it easier to parse, e.g.: > > > GIT_TRACE=1 git stash list > [00:12:10.544266 git.c:512] trace: exec: 'git-stash' 'list' > [00:12:10.544266 run-command.c:337] trace: run_command: 'git-stash' 'list' > [00:12:10.649779 git.c:312] trace: built-in: git 'rev-parse' '--git-dir' This is easier to parse if " " and ":" are found in the names of _our_ source files and "]" isn't, but is that really the case? > Or print file:line at the end (but what about multi-line messages, such as > packet-trace?): > > > GIT_TRACE=1 git stash list > 00:12:10.544266 trace: exec: 'git-stash' 'list' (git.c:512) Somehow I find this a lot harder to read than the other two. I am not commenting on the usefulness of the patch series (yet), though. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 00/11] add performance tracing facility
Here's v5 of the performance tracing patch series, now including a bunch of cleanups and adding timestamp, file and line to all trace output. I'm particularly interested in feedback for the output format. As file names have different lengths, printing file:line as prefix results in unaligned output: > GIT_TRACE=1 git stash list 00:12:10.544266 git.c:512 trace: exec: 'git-stash' 'list' 00:12:10.544266 run-command.c:337 trace: run_command: 'git-stash' 'list' 00:12:10.649779 git.c:312 trace: built-in: git 'rev-parse' '--git-dir' We could add separators to make it easier to parse, e.g.: > GIT_TRACE=1 git stash list [00:12:10.544266 git.c:512] trace: exec: 'git-stash' 'list' [00:12:10.544266 run-command.c:337] trace: run_command: 'git-stash' 'list' [00:12:10.649779 git.c:312] trace: built-in: git 'rev-parse' '--git-dir' Or print file:line at the end (but what about multi-line messages, such as packet-trace?): > GIT_TRACE=1 git stash list 00:12:10.544266 trace: exec: 'git-stash' 'list' (git.c:512) 00:12:10.544266 trace: run_command: 'git-stash' 'list' (run-command.c:337) 00:12:10.649779 trace: built-in: git 'rev-parse' '--git-dir' (git.c:312) Karsten Blees (11): trace: move trace declarations from cache.h to new trace.h trace: consistently name the format parameter trace: remove redundant printf format attribute trace: factor out printing to the trace file trace: add infrastructure to augment trace output with additional info trace: add current timestamp to all trace output trace: move code around, in preparation to file:line output trace: add 'file:line' to all trace output trace: add high resolution timer function to debug performance issues trace: add trace_performance facility to debug performance issues git: add performance tracing for git's main() function to debug scripts Makefile | 7 ++ cache.h | 13 +-- config.mak.uname | 1 + git-compat-util.h | 4 + git.c | 2 + trace.c | 304 +++--- trace.h | 96 + 7 files changed, 379 insertions(+), 48 deletions(-) create mode 100644 trace.h -- 1.9.2.msysgit.0.501.gaeecf09 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
