Commit cf050cc2 authored by ofrobots's avatar ofrobots Committed by Commit bot

Revert of [heap] remove unneeded call to LowerInlineAllocationLimit (patchset...

Revert of [heap] remove unneeded call to LowerInlineAllocationLimit (patchset #2 id:20001 of https://codereview.chromium.org/1390013002/ )

Reason for revert:
Causes memory footprint regression: https://code.google.com/p/chromium/issues/detail?id=541135

The intent of the code here was to advance the inline allocation limit without counting the allocated memory towards a step. Calling LowerInlineAllocationLimit this way is a blunt way of doing it, but it works.

At this point it is simplest to revert this CL. My follow-on CL (https://codereview.chromium.org/1404523002/) can address the 'bluntness' of calling LowerInlineAllocationLimit from here along with leaving a comment about the intent.

revert_cq: 1
revert_reason_textarea: Causes memory footprint regression: https://code.google.com/p/chromium/issues/detail?id=541135

The intent of the code here was to advance the inline allocation limit without counting the allocated memory towards a step. Calling LowerInlineAllocationLimit this way is a blunt way of doing it, but it works.

At this point it is simplest to revert this CL. My follow-on CL (https://codereview.chromium.org/1404523002/) can address the 'bluntness' of calling LowerInlineAllocationLimit from here along with leaving a comment about the intent.

Original issue's description:
> [heap] remove unneeded call to LowerInlineAllocationLimit
>
> Calling LowerInlineAllocationLimit from the bottom of Heap::Scavenge seems to be
> a no-op.
>
>   new_space_.LowerInlineAllocationLimit(
>       new_space_.inline_allocation_limit_step());
>
> LowerInlineAllocatoinLimit does the following things:
>
> 1. Set the inline_allocation_limit_step_ to the passed in value. No-op.
> 2. Calls UpdateInlineAllocationLimit(0). This is unnecessary here as it has
>    already been called when new_space_.ResetAllocationInfo was called above.
> 3. Sets top_on_previous_step_. This again is unnecessary as it gets reached by
>    ResetAllocationInfo as well.
>
> BUG=
> R=hpayer@chromium.org,ulan@chromium.org
>
> Committed: https://crrev.com/9f8e8b835a468b1622c5350a01a97bc32c5b2fb7
> Cr-Commit-Position: refs/heads/master@{#31156}

TBR=hpayer@chromium.org,ulan@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=chromium:541135
LOG=n

Review URL: https://codereview.chromium.org/1405043005

Cr-Commit-Position: refs/heads/master@{#31574}
parent 80bc0803
......@@ -1690,6 +1690,9 @@ void Heap::Scavenge() {
// Set age mark.
new_space_.set_age_mark(new_space_.top());
new_space_.LowerInlineAllocationLimit(
new_space_.inline_allocation_limit_step());
array_buffer_tracker()->FreeDead(true);
// Update how much has survived scavenge.
......
......@@ -2771,6 +2771,10 @@ class NewSpace : public Space {
bool IsFromSpaceCommitted() { return from_space_.is_committed(); }
inline intptr_t inline_allocation_limit_step() {
return inline_allocation_limit_step_;
}
SemiSpace* active_space() { return &to_space_; }
private:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment