Skip to content

Commit

Permalink
Record reasons of OOM intervention failures to UMA
Browse files Browse the repository at this point in the history
We observe that the intervention does not have any effect on population.
Add metrics to see why it fails.

BUG=843419

Change-Id: I1e7443a749331135dbbad9c51ff79ba9d1975db7
Reviewed-on: https://chromium-review.googlesource.com/1083594
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564330}
  • Loading branch information
ssiddhartha authored and Commit Bot committed Jun 5, 2018
1 parent 3ecf97c commit 12d9d3c
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 6 deletions.
21 changes: 19 additions & 2 deletions chrome/browser/android/oom_intervention/oom_intervention_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "base/feature_list.h"
#include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_number_conversions.h"
#include "base/sys_info.h"
#include "chrome/common/chrome_features.h"
Expand Down Expand Up @@ -95,6 +96,15 @@ bool GetSwapFreeThreshold(uint64_t* threshold) {
return true;
}

// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class OomInterventionBrowserMonitorStatus {
kEnabledWithValidConfig = 0,
kDisabledWithInvalidParam = 1,
kDisabledWithNoSwap = 2,
kMaxValue = kDisabledWithNoSwap
};

} // namespace

OomInterventionConfig::OomInterventionConfig()
Expand All @@ -114,10 +124,17 @@ OomInterventionConfig::OomInterventionConfig()

// Enable intervention only if at least one threshold is set for detection
// in each process.
if (!GetSwapFreeThreshold(&swapfree_threshold_) ||
!GetRendererMemoryThresholds(&renderer_detection_args_)) {
OomInterventionBrowserMonitorStatus status =
OomInterventionBrowserMonitorStatus::kEnabledWithValidConfig;
if (!GetSwapFreeThreshold(&swapfree_threshold_)) {
is_intervention_enabled_ = false;
status = OomInterventionBrowserMonitorStatus::kDisabledWithNoSwap;
} else if (!GetRendererMemoryThresholds(&renderer_detection_args_)) {
is_intervention_enabled_ = false;
status = OomInterventionBrowserMonitorStatus::kDisabledWithInvalidParam;
}
UMA_HISTOGRAM_ENUMERATION(
"Memory.Experimental.OomIntervention.BrowserMonitorStatus", status);
}

// static
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void RecordInterventionStateOnCrash(bool accepted) {

// static
bool OomInterventionTabHelper::IsEnabled() {
return NearOomMonitor::GetInstance() != nullptr;
return OomInterventionConfig::GetInstance()->is_intervention_enabled();
}

OomInterventionTabHelper::OomInterventionTabHelper(
Expand Down
26 changes: 23 additions & 3 deletions third_party/blink/renderer/controller/oom_intervention_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <fcntl.h>
#include <unistd.h>

#include "base/metrics/histogram_macros.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/platform/bindings/v8_per_isolate_data.h"
Expand Down Expand Up @@ -79,6 +80,20 @@ uint64_t BlinkMemoryWorkloadCaculator() {
return v8_size + blink_gc_size + partition_alloc_size;
}

// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class RendererInterventionEnabledStatus {
kDetectionOnlyEnabled = 0,
kTriggerEnabled = 1,
kDisabledFailedMemoryMetricsFetch = 2,
kMaxValue = kDisabledFailedMemoryMetricsFetch
};

void RecordEnabledStatus(RendererInterventionEnabledStatus status) {
UMA_HISTOGRAM_ENUMERATION(
"Memory.Experimental.OomIntervention.RendererEnabledStatus", status);
}

} // namespace

// static
Expand Down Expand Up @@ -109,10 +124,15 @@ void OomInterventionImpl::StartDetection(
if (!status_fd_.is_valid())
status_fd_.reset(open("/proc/self/status", O_RDONLY));
// Disable intervention if we cannot get memory details of current process.
// TODO(ssid): Add UMA here to make sure we don't stop disable intervention
// more often than expected.
if (!statm_fd_.is_valid() || !status_fd_.is_valid())
if (!statm_fd_.is_valid() || !status_fd_.is_valid()) {
RecordEnabledStatus(
RendererInterventionEnabledStatus::kDisabledFailedMemoryMetricsFetch);
return;
}
RecordEnabledStatus(
trigger_intervention
? RendererInterventionEnabledStatus::kTriggerEnabled
: RendererInterventionEnabledStatus::kDetectionOnlyEnabled);

detection_args_ = std::move(detection_args);
trigger_intervention_ = trigger_intervention;
Expand Down
12 changes: 12 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34438,6 +34438,18 @@ Called by update_net_trust_anchors.py.-->
<int value="3" label="(non-invalidated) replies received"/>
</enum>

<enum name="OomInterventionBrowserMonitorStatus">
<int value="0" label="Enabled with a valid config"/>
<int value="1" label="Disabled since bad config param"/>
<int value="2" label="Disabled since device has no swap"/>
</enum>

<enum name="OomInterventionRendererStatus">
<int value="0" label="Detection only enabled on renderer"/>
<int value="1" label="Detection with trigger enabled on renderer"/>
<int value="2" label="Disabled since renderer cannot fetch memory total"/>
</enum>

<enum name="OomInterventionUserDecision">
<int value="0" label="Declined"/>
<int value="1" label="Accepted"/>
Expand Down
20 changes: 20 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41890,6 +41890,16 @@ uploading your change for review.
</summary>
</histogram>

<histogram name="Memory.Experimental.OomIntervention.BrowserMonitorStatus"
enum="OomInterventionBrowserMonitorStatus" expires_after="M70">
<owner>ssid@chromium.org</owner>
<summary>
Records the status of OOM intervention if the feature is turned on, with
various reasons for failure, in the browser process. Recorded once at
browser startup.
</summary>
</histogram>

<histogram name="Memory.Experimental.OomIntervention.InterventionStateOnCrash"
enum="OomInterventionUserDecision">
<owner>bashi@chromium.org</owner>
Expand Down Expand Up @@ -41938,6 +41948,16 @@ uploading your change for review.
</summary>
</histogram>

<histogram name="Memory.Experimental.OomIntervention.RendererEnabledStatus"
enum="OomInterventionRendererStatus" expires_after="M70">
<owner>ssid@chromium.org</owner>
<summary>
Records the status of OOM intervention in renderer process if the feature is
turned on, with various reasons for failure. Recorded at each navigation or
reload.
</summary>
</histogram>

<histogram
name="Memory.Experimental.OomIntervention.RendererGoneAfterDetectionTime"
units="ms">
Expand Down

0 comments on commit 12d9d3c

Please sign in to comment.