Re: [PATCH] InstanceId formatting

On 10/14/2010 04:53 PM, Kinkie wrote:
> Hi all,
> this patch changes slightly the formatting of InstanceId's output so
> that it's easier to parse and isolate.
> Opinions?
>
> === modified file 'src/base/InstanceId.h'
> --- src/base/InstanceId.h 2010-10-04 14:49:57 +0000
> +++ src/base/InstanceId.h 2010-10-14 22:33:00 +0000
> @@ -43,7 +43,7 @@
> #define InstanceIdDefinitions(Class, prefix) \
> template<> std::ostream& \
> InstanceId<Class>::print(std::ostream&os) const { \
> - return os<< Prefix<< value; \
> + return os<< '['<< Prefix<< '#'<< value<< ']'; \
> } \
> template<> const char *InstanceId<Class>::Prefix = prefix; \
> template<> InstanceId<Class>::Value InstanceId<Class>::Last = 0

-1. This will break formatting for all current and future Jobs that use
status() method, which adds its own [...] wrapping and includes job ID
inside those brackets. And I do not think '#' addition will make logged
IDs more unique, but it will waste space.

Correctly prefixed IDs are already fairly easy to find because they
should use fairly unique prefixes followed by a number. The existing
transaction tracing scripts already find such IDs, and I do not recall
seeing false positives.

BTW, ease of search is the reason why a prefix like "MemBlob" is
slightly worse than "blob". HERE will produce many "MemBlob" strings but
no "blob" strings. Regular debugging comments may use blob, but should
not use blob followed by a number.

Sorry,

Alex.
Received on Tue Oct 19 2010 - 17:21:31 MDT