Grab crash handler's peak commit charge in fallback handler

Grab crash handler's peak commit charge in fallback handler

This is a follow up to https://crrev.com/2643273002.

BUG=683158
Review-Url: https://codereview.chromium.org/2653953002
Cr-Commit-Position: refs/heads/master@{#446016}
Committed: https://chromium.googlesource.com/chromium/src/+/3175abc1ac46d878e39af3e5956436d48d5518fb

[manzagop (departed), 2017-01-24 16:30:13]

The CQ bit was checked by manzagop@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 16:30:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[manzagop (departed), 2017-01-24 16:32:18]

manzagop@chromium.org changed reviewers:
+ jochen@chromium.org

[manzagop (departed), 2017-01-24 16:32:18]

Hi Jochen!

This is a follow up to Siggi's CL. PeakWorkingSetSize is likely diminished due
to the low mem situation, so we want to look at PeakPagefileUsage.

Thanks!

Pierre

[commit-bot: I haz the power, 2017-01-24 16:34:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 16:34:04]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[manzagop (departed), 2017-01-24 18:45:32]

For reference, the trybot issue https://crbug.com/684573

[manzagop (departed), 2017-01-24 20:35:32]

The CQ bit was checked by manzagop@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 20:36:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 20:47:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 20:47:12]

Dry run: This issue passed the CQ dry run.

[Sigurður Ásgeirsson, 2017-01-25 08:25:46]

Scottmg might be a better reviewer for this. I know Jochen is also
traveling this week. I just tapped him because Scott was incommunicado.

On Tue, Jan 24, 2017, 17:32 <manzagop@chromium.org> wrote:

> Reviewers: jochen
> CL: https://codereview.chromium.org/2653953002/
>
> Message:
> Hi Jochen!
>
> This is a follow up to Siggi's CL. PeakWorkingSetSize is likely diminished
> due
> to the low mem situation, so we want to look at PeakPagefileUsage.
>
> Thanks!
>
> Pierre
>
> Description:
> Grab crash handler's peak commit charge in fallback handler
>
> This is a follow up to https://crrev.com/2643273002.
>
> BUG=683158
>
> Affected files (+18, -1 lines):
> M components/crash/content/app/fallback_crash_handler_win.cc
> M components/crash/content/app/fallback_crash_handler_win_unittest.cc
>
>
> Index: components/crash/content/app/fallback_crash_handler_win.cc
> diff --git a/components/crash/content/app/fallback_crash_handler_win.cc
> b/components/crash/content/app/fallback_crash_handler_win.cc
> index
>
a554673f6a606f2228480d3f15deba7ed1c949cc..2e3310d874dd335f9dfb607fad22f6bf481b198b
> 100644
> --- a/components/crash/content/app/fallback_crash_handler_win.cc
> +++ b/components/crash/content/app/fallback_crash_handler_win.cc
> @@ -9,6 +9,8 @@
>
> #include <algorithm>
> #include <map>
> +#include <memory>
> +#include <string>
> #include <vector>
>
> #include "base/command_line.h"
> @@ -52,6 +54,10 @@ void AcquireMemoryMetrics(const base::Process& process,
> crash_keys->insert(std::make_pair(
> "ProcessPeakWorkingSetSize",
> base::Uint64ToString(process_memory.PeakWorkingSetSize / kPageSize)));
> +
> + crash_keys->insert(std::make_pair(
> + "ProcessPeakPagefileUsage",
> + base::Uint64ToString(process_memory.PeakPagefileUsage / kPageSize)));
> }
>
> // Grab system commit memory. Also best effort.
> Index: components/crash/content/app/fallback_crash_handler_win_unittest.cc
> diff --git
> a/components/crash/content/app/fallback_crash_handler_win_unittest.cc
> b/components/crash/content/app/fallback_crash_handler_win_unittest.cc
> index
>
cdd68d80da8a3af223bd4efea81b12082ef867ab..13ca131f0204c80ae3b82f4d5e95d0649be88b0d
> 100644
> --- a/components/crash/content/app/fallback_crash_handler_win_unittest.cc
> +++ b/components/crash/content/app/fallback_crash_handler_win_unittest.cc
> @@ -4,6 +4,11 @@
>
> #include "components/crash/content/app/fallback_crash_handler_win.h"
>
> +#include <map>
> +#include <memory>
> +#include <string>
> +#include <vector>
> +
> #include "base/base_switches.h"
> #include "base/command_line.h"
> #include "base/files/file_path.h"
> @@ -116,7 +121,7 @@ class FallbackCrashHandlerWinTest : public
> testing::Test {
>
> std::string SelfHandleAsString() const {
> return base::UintToString(base::win::HandleToUint32(self_handle_));
> - };
> + }
>
> void CreateDatabase() {
> std::unique_ptr<crashpad::CrashReportDatabase> database =
> @@ -128,6 +133,7 @@ class FallbackCrashHandlerWinTest : public
> testing::Test {
> base::ProcessHandle self_handle_;
> base::ScopedTempDir database_dir_;
>
> + private:
> DISALLOW_COPY_AND_ASSIGN(FallbackCrashHandlerWinTest);
> };
>
> @@ -239,6 +245,11 @@ TEST_F(FallbackCrashHandlerWinTest,
> GenerateCrashDump) {
> EXPECT_TRUE(base::StringToUint64(it->second, &int_value));
> EXPECT_NE(0U, int_value);
>
> + it = parameters.find("ProcessPeakPagefileUsage");
> + EXPECT_NE(parameters.end(), it);
> + EXPECT_TRUE(base::StringToUint64(it->second, &int_value));
> + EXPECT_NE(0U, int_value);
> +
> it = parameters.find("SystemCommitRemaining");
> EXPECT_NE(parameters.end(), it);
> EXPECT_TRUE(base::StringToUint64(it->second, &int_value));
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jochen (gone - plz use gerrit), 2017-01-25 12:23:43]

lgtm

[manzagop (departed), 2017-01-25 14:41:18]

Double plus thanks in that case!

[manzagop (departed), 2017-01-25 14:41:24]

The CQ bit was checked by manzagop@chromium.org

[commit-bot: I haz the power, 2017-01-25 14:41:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-25 14:45:17]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1485355284045370, "parent_rev":
"6fc1c64c630cc76c40dc635cb4d39d3d28ee616b", "commit_rev":
"3175abc1ac46d878e39af3e5956436d48d5518fb"}

[commit-bot: I haz the power, 2017-01-25 14:45:56]

Description was changed from

==========
Grab crash handler's peak commit charge in fallback handler

This is a follow up to https://crrev.com/2643273002.

BUG=683158
==========

to

==========
Grab crash handler's peak commit charge in fallback handler

This is a follow up to https://crrev.com/2643273002.

BUG=683158

Review-Url: https://codereview.chromium.org/2653953002
Cr-Commit-Position: refs/heads/master@{#446016}
Committed:
https://chromium.googlesource.com/chromium/src/+/3175abc1ac46d878e39af3e59564...
==========

[commit-bot: I haz the power, 2017-01-25 14:45:57]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/3175abc1ac46d878e39af3e59564...