Lars Volker has posted comments on this change.

Change subject: IMPALA-4226, IMPALA-4227: bump max threads, handle dwz 
compressed symbols
......................................................................


Patch Set 1:

(2 comments)

Thanks for the review. Please see my inline comments.

http://gerrit.cloudera.org:8080/#/c/6461/1/source/breakpad/breakpad-88e5b2c8806bac3f2c80d2fe80094be5bd371601-patches/0002-Bump-max_threads-and-max_regions.patch
File 
source/breakpad/breakpad-88e5b2c8806bac3f2c80d2fe80094be5bd371601-patches/0002-Bump-max_threads-and-max_regions.patch:

Line 18: -uint32_t MinidumpThreadList::max_threads_ = 4096;
> Should we try and get this fixed upstream? E.g. by making it a command-line
Good point, I pinged the breakpad dev list and suggested doing this. In that 
case we would need to bump our version, so it would require an additional 
change to the toolchain. Would you be ok with keeping this here as a workaround 
until it is fixed upstream?


Line 19: +uint32_t MinidumpThreadList::max_threads_ = 32786;
> Is this enough? I believe sometimes we increase this further.
I haven't seen minidumps with more threads in the wild but I'm happy to bump 
this to a larger number. I don't know what else will break if this gets too 
large, so I tried to stay somewhat conservative here. Do you have a suggestion?


-- 
To view, visit http://gerrit.cloudera.org:8080/6461
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1bf83edd8cda037c31a842801ad1445f3fd4f71e
Gerrit-PatchSet: 1
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to