impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [native-toolchain-CR] IMPALA-4226, IMPALA-4227: bump max threads, handle dwz compressed symbols
Date Fri, 24 Mar 2017 12:53:40 GMT
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 <lv@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message