Fix bug in mergetraces.py.

Fix bug in mergetraces.py.

The unit test was unfortunately written in a way that didn't catch it.

BUG=376793
R=pasko@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275390

[pasko, 2014-06-06 09:14:15]

https://codereview.chromium.org/319053004/diff/1/tools/cygprofile/mergetraces...
File tools/cygprofile/mergetraces_unittest.py (right):

https://codereview.chromium.org/319053004/diff/1/tools/cygprofile/mergetraces...
tools/cygprofile/mergetraces_unittest.py:30: (150, 12, '2001:2001', 0x6),
please make sure each tid uniquely identifies the pid in the test

[Philippe, 2014-06-06 09:30:48]

PTAL :)

https://codereview.chromium.org/319053004/diff/1/tools/cygprofile/mergetraces...
File tools/cygprofile/mergetraces_unittest.py (right):

https://codereview.chromium.org/319053004/diff/1/tools/cygprofile/mergetraces...
tools/cygprofile/mergetraces_unittest.py:30: (150, 12, '2001:2001', 0x6),
On 2014/06/06 09:14:15, pasko wrote:
> please make sure each tid uniquely identifies the pid in the test

Done.

[pasko, 2014-06-06 09:41:44]

https://codereview.chromium.org/319053004/diff/40001/tools/cygprofile/mergetr...
File tools/cygprofile/mergetraces.py (right):

https://codereview.chromium.org/319053004/diff/40001/tools/cygprofile/mergetr...
tools/cygprofile/mergetraces.py:161: # Make sure that thread IDs are unique
since this a property we rely on.
s/this/this is/

https://codereview.chromium.org/319053004/diff/40001/tools/cygprofile/mergetr...
tools/cygprofile/mergetraces.py:165: raise Exception('Thread IDs should be
unique.')
in this context we actually have the tid and two pids, would be useful to print
them as part of the message.

This looks simpler:

tid_to_pids_map.setdefault(tid, pid)
if tid_to_pids_map[tid] != pid:
  raise Exception('...')

[Philippe, 2014-06-06 09:50:24]

https://codereview.chromium.org/319053004/diff/40001/tools/cygprofile/mergetr...
File tools/cygprofile/mergetraces.py (right):

https://codereview.chromium.org/319053004/diff/40001/tools/cygprofile/mergetr...
tools/cygprofile/mergetraces.py:161: # Make sure that thread IDs are unique
since this a property we rely on.
On 2014/06/06 09:41:44, pasko wrote:
> s/this/this is/

Done.

https://codereview.chromium.org/319053004/diff/40001/tools/cygprofile/mergetr...
tools/cygprofile/mergetraces.py:161: # Make sure that thread IDs are unique
since this a property we rely on.
On 2014/06/06 09:41:44, pasko wrote:
> s/this/this is/

Done.

https://codereview.chromium.org/319053004/diff/40001/tools/cygprofile/mergetr...
tools/cygprofile/mergetraces.py:165: raise Exception('Thread IDs should be
unique.')
On 2014/06/06 09:41:44, pasko wrote:
> in this context we actually have the tid and two pids, would be useful to
print
> them as part of the message.
> 
> This looks simpler:
> 
> tid_to_pids_map.setdefault(tid, pid)
> if tid_to_pids_map[tid] != pid:
>   raise Exception('...')

Yeah :) I haven't slept much last night :)

[pasko, 2014-06-06 09:53:29]

LGTM!

https://codereview.chromium.org/319053004/diff/30003/tools/cygprofile/mergetr...
File tools/cygprofile/mergetraces.py (right):

https://codereview.chromium.org/319053004/diff/30003/tools/cygprofile/mergetr...
tools/cygprofile/mergetraces.py:161: if tid_to_pid_map.setdefault(tid, pid) !=
pid:
nice!

https://codereview.chromium.org/319053004/diff/30003/tools/cygprofile/mergetr...
tools/cygprofile/mergetraces.py:163: 'Seen PIDs %d and %d for TID=%d. Thread-IDs
should be unique' % (
nit:

s/should be unique/must be unique./

[Philippe, 2014-06-06 10:53:36]

Thanks, Egor!

https://codereview.chromium.org/319053004/diff/30003/tools/cygprofile/mergetr...
File tools/cygprofile/mergetraces.py (right):

https://codereview.chromium.org/319053004/diff/30003/tools/cygprofile/mergetr...
tools/cygprofile/mergetraces.py:163: 'Seen PIDs %d and %d for TID=%d. Thread-IDs
should be unique' % (
On 2014/06/06 09:53:29, pasko wrote:
> nit:
> 
> s/should be unique/must be unique./

Done.

[Philippe, 2014-06-06 10:54:58]

Committed patchset #4 manually as r275390.