Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2903733003:
memory-infra: LOG(FATAL) on unsupported heap profiler configs (Closed)
|
Created:
3 years, 7 months ago by Primiano Tucci (use gerrit) Modified:
3 years, 7 months ago CC:
chromium-reviews, tracing+reviews_chromium.org, wfh+watch_chromium.org, danakj+watch_chromium.org, vmpstr+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Descriptionmemory-infra: LOG(FATAL) on unsupported heap profiler configs
Adds a fatal error when trying to use the heap profiler in a
configuration where it cannot work (e.g., Win debug or component
builds, or anything that doesn't have the shim)
BUG=705461
Review-Url: https://codereview.chromium.org/2903733003
Cr-Commit-Position: refs/heads/master@{#474468}
Committed: https://chromium.googlesource.com/chromium/src/+/59c3060ac761de2221eea6febe7be5d223f0266a
Patch Set 1 #Patch Set 2 : unicode space, grr #Patch Set 3 : unicode space, AGAIN #
Total comments: 2
Messages
Total messages: 25 (18 generated)
The CQ bit was checked by primiano@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== memory-infra: LOG(FATAL) for unsupported heap profiler configs Adds a fatal error when trying to use the heap profiler in a configuration where it cannot work (e.g., Win debug or component builds, or anything that doesn't have the shim) BUG=705461 ========== to ========== memory-infra: LOG(FATAL) on unsupported heap profiler configs Adds a fatal error when trying to use the heap profiler in a configuration where it cannot work (e.g., Win debug or component builds, or anything that doesn't have the shim) BUG=705461 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by primiano@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by primiano@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
primiano@chromium.org changed reviewers: + erikchen@chromium.org
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by primiano@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
wez@chromium.org changed reviewers: + wez@chromium.org
https://codereview.chromium.org/2903733003/diff/40001/base/trace_event/memory... File base/trace_event/memory_dump_manager.cc (right): https://codereview.chromium.org/2903733003/diff/40001/base/trace_event/memory... base/trace_event/memory_dump_manager.cc:204: #if !BUILDFLAG(USE_EXPERIMENTAL_ALLOCATOR_SHIM) Do we ever actually set this under Windows? The Windows shim code actually checks for defined(ALLOCATOR_SHIM) (see https://cs.chromium.org/chromium/src/base/allocator/allocator_shim_win.cc?l=10), which I think depends on win_allocator_shim in GN. (Perhaps I'm mis-reading the Windows shim case, though);
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495661010284050,
"parent_rev": "fbe49a4a05a04c51bc6ac3be5765c59740029192", "commit_rev":
"59c3060ac761de2221eea6febe7be5d223f0266a"}
Message was sent while issue was closed.
Description was changed from ========== memory-infra: LOG(FATAL) on unsupported heap profiler configs Adds a fatal error when trying to use the heap profiler in a configuration where it cannot work (e.g., Win debug or component builds, or anything that doesn't have the shim) BUG=705461 ========== to ========== memory-infra: LOG(FATAL) on unsupported heap profiler configs Adds a fatal error when trying to use the heap profiler in a configuration where it cannot work (e.g., Win debug or component builds, or anything that doesn't have the shim) BUG=705461 Review-Url: https://codereview.chromium.org/2903733003 Cr-Commit-Position: refs/heads/master@{#474468} Committed: https://chromium.googlesource.com/chromium/src/+/59c3060ac761de2221eea6febe7b... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/59c3060ac761de2221eea6febe7b...
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2902343002/ by wez@chromium.org. The reason for reverting is: As noted in my comment on Patch Set #3, this disables heap-profiling on Windows, even in Static builds..
Message was sent while issue was closed.
> this disables heap-profiling on Windows, even in Static builds.. Can I ask you your gn args? I just tried and this actually works as I intended on Windows. Works for me with: ----- is_component_build=false is_debug=false ------ and fails with either ---- is_component_build=false is_debug=true ---- is_component_build=true is_debug=false ---- https://codereview.chromium.org/2903733003/diff/40001/base/trace_event/memory... File base/trace_event/memory_dump_manager.cc (right): https://codereview.chromium.org/2903733003/diff/40001/base/trace_event/memory... base/trace_event/memory_dump_manager.cc:204: #if !BUILDFLAG(USE_EXPERIMENTAL_ALLOCATOR_SHIM) On 2017/05/24 21:29:25, Wez wrote: > Do we ever actually set this under Windows? I thought so: From build/config/allocator.gni: if ((is_linux || is_android || is_mac ||(is_win && !is_component_build && !is_debug)) && !is_asan && !is_lsan && !is_tsan && !is_msan) { _default_use_experimental_allocator_shim = true then use_experimental_allocator_shim = _default_use_experimental_allocator_shim then in allocator_features.h "USE_EXPERIMENTAL_ALLOCATOR_SHIM=$use_experimental_allocator_shim", The Windows shim code actually > checks for defined(ALLOCATOR_SHIM) (see > https://cs.chromium.org/chromium/src/base/allocator/allocator_shim_win.cc?l=10), > which I think depends on win_allocator_shim in GN. That should be the old shim (Which I am going to kill soon), from the comment: # The Windows-only allocator shim is only enabled for Release static builds, and # is mutually exclusive with the generalized shim. win_use_allocator_shim = is_win && !is_component_build && !is_debug && !use_experimental_allocator_shim && !is_asan note the: "!use_experimental_allocator_shim" |
