Skip to content

Commit

Permalink
Revert "Reland "Use thresholds based on RAM size for OOM intervention""
Browse files Browse the repository at this point in the history
This reverts commit 9191004.

Reason for revert: The failure after the revert was a different failure.
Sorry for the reland.

Original change's description:
> Reland "Use thresholds based on RAM size for OOM intervention"
> 
> This reverts commit 50257b9.
> 
> Reason for revert: This was not the cause of failure, see
> https://crbug.com/845763
> 
> Original change's description:
> > Revert "Use thresholds based on RAM size for OOM intervention"
> > 
> > This reverts commit 0c2124b.
> > 
> > Reason for revert: Speculatively reverting to see if it fixes webgl conformance test timeouts
> > Bug: 845411
> > 
> > Original change's description:
> > > Use thresholds based on RAM size for OOM intervention
> > > 
> > > This CL does not change behavior of existing experiment on 512 devices.
> > > 1. Remove low-end device checks while enabling intervention. This can be
> > >    filtered in Finch config.
> > > 2. Set threshold based on percentage of RAM instead of absolute number.
> > > 3. Disable intervention if device has more than 512MB RAM and threshold
> > >    was set to absolute number.
> > > 
> > > BUG=843419
> > > 
> > > 
> > > Change-Id: Ib40a038d9d2f111a514b3f41ce7f32a3ea3a156d
> > > Reviewed-on: https://chromium-review.googlesource.com/1066804
> > > Commit-Queue: Siddhartha S <ssid@chromium.org>
> > > Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#560430}
> > 
> > TBR=bashi@chromium.org,ssid@chromium.org
> > 
> > Change-Id: I4b50e522dcc6c91542927a9e769e5eee24f486a5
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 843419
> > Reviewed-on: https://chromium-review.googlesource.com/1069567
> > Reviewed-by: enne <enne@chromium.org>
> > Commit-Queue: enne <enne@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#560761}
> 
> TBR=bashi@chromium.org,enne@chromium.org,ssid@chromium.org
> 
> Change-Id: I4d0a08cd44772dc690ac1d93b1d92b4934ed0373
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 845411, 843419
> Reviewed-on: https://chromium-review.googlesource.com/1069888
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Commit-Queue: Siddhartha S <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#560882}

TBR=bashi@chromium.org,enne@chromium.org,ssid@chromium.org

Change-Id: Ib8841dddaa2a167e674c997ec1577036df26e8cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 845411, 843419
Reviewed-on: https://chromium-review.googlesource.com/1069876
Reviewed-by: Siddhartha S <ssid@chromium.org>
Commit-Queue: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560889}
  • Loading branch information
ssiddhartha authored and Commit Bot committed May 23, 2018
1 parent 2f65eec commit a21bf6f
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 50 deletions.
41 changes: 4 additions & 37 deletions chrome/browser/android/oom_intervention/near_oom_monitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include "base/feature_list.h"
#include "base/metrics/field_trial_params.h"
#include "base/strings/string_number_conversions.h"
#include "base/sys_info.h"
#include "base/threading/thread_task_runner_handle.h"
#include "chrome/common/chrome_features.h"
Expand All @@ -16,10 +15,6 @@ namespace {

const char kSwapFreeThresholdRatioParamName[] = "swap_free_threshold_ratio";
const char kUseComponentCallbacks[] = "use_component_callbacks";
const char kRendererWorkloadThresholdDeprecated[] =
"renderer_workload_threshold";
const char kRendererWorkloadThresholdPercentage[] =
"renderer_workload_threshold_percentage";

// Default SwapFree/SwapTotal ratio for detecting near-OOM situation.
// TODO(bashi): Confirm that this is appropriate.
Expand All @@ -33,36 +28,14 @@ constexpr base::TimeDelta kDefaultMonitoringDelta =
constexpr base::TimeDelta kDefaultCooldownDelta =
base::TimeDelta::FromSeconds(30);

uint64_t GetRendererMemoryWorkloadThreshold() {
// Approximately 80MB for 512MB devices.
const uint64_t kDefaultMemoryWorkloadThresholdPercent = 16;

std::string threshold_str = base::GetFieldTrialParamValueByFeature(
features::kOomIntervention, kRendererWorkloadThresholdPercentage);
uint64_t threshold = 0;
size_t ram_size = base::SysInfo::AmountOfPhysicalMemory();
if (base::StringToUint64(threshold_str, &threshold)) {
return threshold * ram_size / 100;
}

// If the old trigger param is set, then enable intervention only on 512MB
// devices.
threshold_str = base::GetFieldTrialParamValueByFeature(
features::kOomIntervention, kRendererWorkloadThresholdDeprecated);
if (base::StringToUint64(threshold_str, &threshold)) {
if (ram_size > 512 * 1024 * 1024)
return 0;
return threshold;
}
return kDefaultMemoryWorkloadThresholdPercent * ram_size / 100;
}

} // namespace

// static
NearOomMonitor* NearOomMonitor::Create() {
if (!base::FeatureList::IsEnabled(features::kOomIntervention))
return nullptr;
if (!base::SysInfo::IsLowEndDevice())
return nullptr;

base::SystemMemoryInfoKB memory_info;
if (!base::GetSystemMemoryInfo(&memory_info))
Expand All @@ -78,12 +51,8 @@ NearOomMonitor* NearOomMonitor::Create() {
kDefaultSwapFreeThresholdRatio);
int64_t swapfree_threshold =
static_cast<int64_t>(memory_info.swap_total * threshold_ratio);

uint64_t renderer_workload_threshold = GetRendererMemoryWorkloadThreshold();
if (!renderer_workload_threshold)
return nullptr;
return new NearOomMonitor(base::ThreadTaskRunnerHandle::Get(),
swapfree_threshold, renderer_workload_threshold);
swapfree_threshold);
}

// static
Expand All @@ -94,15 +63,13 @@ NearOomMonitor* NearOomMonitor::GetInstance() {

NearOomMonitor::NearOomMonitor(
scoped_refptr<base::SequencedTaskRunner> task_runner,
int64_t swapfree_threshold,
uint64_t renderer_workload_threshold)
int64_t swapfree_threshold)
: task_runner_(task_runner),
check_callback_(
base::Bind(&NearOomMonitor::Check, base::Unretained(this))),
monitoring_interval_(kDefaultMonitoringDelta),
cooldown_interval_(kDefaultCooldownDelta),
swapfree_threshold_(swapfree_threshold),
renderer_workload_threshold_(renderer_workload_threshold),
component_callback_is_enabled_(
base::GetFieldTrialParamByFeatureAsBool(features::kOomIntervention,
kUseComponentCallbacks,
Expand Down
8 changes: 1 addition & 7 deletions chrome/browser/android/oom_intervention/near_oom_monitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,11 @@ class NearOomMonitor {
void OnLowMemory(JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller);

uint64_t renderer_workload_threshold() const {
return renderer_workload_threshold_;
}

protected:
static NearOomMonitor* Create();

NearOomMonitor(scoped_refptr<base::SequencedTaskRunner> task_runner,
int64_t swapfree_threshold,
uint64_t renderer_workload_threshold);
int64_t swapfree_threshold);

// Gets system memory info. This is a virtual method so that we can override
// this for testing.
Expand Down Expand Up @@ -77,7 +72,6 @@ class NearOomMonitor {
base::TimeTicks next_check_time_;

int64_t swapfree_threshold_;
uint64_t renderer_workload_threshold_;

CallbackList callbacks_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
namespace {
const int64_t kTestSwapTotalKB = 128 * 1024;
const int64_t kTestSwapFreeThreshold = kTestSwapTotalKB / 4;
const uint64_t kRendererWorkloadThreshold = 10 * 1024;
} // namespace

class MockNearOomMonitor : public NearOomMonitor {
public:
explicit MockNearOomMonitor(
scoped_refptr<base::SequencedTaskRunner> task_runner)
: NearOomMonitor(task_runner,
kTestSwapFreeThreshold,
kRendererWorkloadThreshold) {
: NearOomMonitor(task_runner, kTestSwapFreeThreshold) {
// Start with 128MB swap total and 64MB swap free.
memory_info_.swap_total = kTestSwapTotalKB;
memory_info_.swap_free = kTestSwapTotalKB / 2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ void RecordInterventionStateOnCrash(bool accepted) {
// Field trial parameter names.
const char kRendererPauseParamName[] = "pause_renderer";
const char kShouldDetectInRenderer[] = "detect_in_renderer";
const char kRendererWorkloadThreshold[] = "renderer_workload_threshold";

bool RendererPauseIsEnabled() {
static bool enabled = base::GetFieldTrialParamByFeatureAsBool(
Expand All @@ -74,6 +75,18 @@ bool ShouldDetectInRenderer() {
return enabled;
}

uint64_t GetRendererMemoryWorkloadThreshold() {
const uint64_t kDefaultMemoryWorkloadThreshold = 80 * 1024 * 1024;

std::string threshold_str = base::GetFieldTrialParamValueByFeature(
features::kOomIntervention, kRendererWorkloadThreshold);
uint64_t threshold = 0;
if (!base::StringToUint64(threshold_str, &threshold)) {
return kDefaultMemoryWorkloadThreshold;
}
return threshold;
}

} // namespace

// static
Expand All @@ -87,8 +100,7 @@ OomInterventionTabHelper::OomInterventionTabHelper(
decider_(OomInterventionDecider::GetForBrowserContext(
web_contents->GetBrowserContext())),
binding_(this),
renderer_memory_workload_threshold_(
NearOomMonitor::GetInstance()->renderer_workload_threshold()),
renderer_memory_workload_threshold_(GetRendererMemoryWorkloadThreshold()),
weak_ptr_factory_(this) {
OutOfMemoryReporter::FromWebContents(web_contents)->AddObserver(this);
shared_metrics_buffer_ = base::UnsafeSharedMemoryRegion::Create(
Expand Down

0 comments on commit a21bf6f

Please sign in to comment.