Fix free space repairing after deserialization.

Fix free space repairing after deserialization.

Review-Url: https://codereview.chromium.org/2806463002
Cr-Commit-Position: refs/heads/master@{#44455}
Committed: https://chromium.googlesource.com/v8/v8/+/9ce9dde499792396a3997985f0867d5c92d7b18c

[Slava Chigrin, 2017-04-06 12:26:36]

vchigrin@yandex-team.ru changed reviewers:
+ yangguo@chromium.org

[Slava Chigrin, 2017-04-06 12:30:13]

https://codereview.chromium.org/2806463002/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2806463002/diff/1/src/heap/spaces.cc#newcode2842
src/heap/spaces.cc:2842: Address address =
I suppose here was a bug. As described here
https://cs.chromium.org/chromium/src/v8/src/heap/spaces.cc?gsn=AllocatePage&l...
Executable pages have a guard area at the end, that was not taken into account
at creating filler objects.

If I made a mistake, please, correct me.

[ulan, 2017-04-06 15:11:26]

ulan@chromium.org changed reviewers:
+ ulan@chromium.org

[ulan, 2017-04-06 15:11:27]

Thanks for uploading the fix. Using kPageSize seems indeed to be a bug.

https://codereview.chromium.org/2806463002/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2806463002/diff/1/src/heap/spaces.cc#newcode2842
src/heap/spaces.cc:2842: Address address =
We can use page->HighWaterMark() and page->area_end() which would be more
robust.

Could you please rewrite the code starting from line 2836 to something like:

  // Each page may have a small free space that is not tracked by a free list.
  // Those free spaces still contain null as their map pointer.
  // Overwrite them with new fillers.
  for (Page* page : *this) {
    int size = static_cast<int>(page->wasted_memory());
    if (size == 0) {
      // If there is no wasted memory then all free space is in the free list.
      continue;
    }
    Address start = page->HighWaterMark();
    Address end = page->area_end();
    CHECK_EQ(size, static_cast<int>(end - start));
    heap()->CreateFillerObjectAt(start, end, ClearRecordedSlots::kNo);
  }

[Slava Chigrin, 2017-04-06 15:48:34]

Thank you for review, uploaded new patch (fixed tiny compilation error in last
line of your snippet, everything is the same).

https://codereview.chromium.org/2806463002/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (right):

https://codereview.chromium.org/2806463002/diff/1/src/heap/spaces.cc#newcode2842
src/heap/spaces.cc:2842: Address address =
On 2017/04/06 15:11:27, ulan wrote:
> We can use page->HighWaterMark() and page->area_end() which would be more
> robust.
> 
> Could you please rewrite the code starting from line 2836 to something like:
> 
>   // Each page may have a small free space that is not tracked by a free list.
>   // Those free spaces still contain null as their map pointer.
>   // Overwrite them with new fillers.
>   for (Page* page : *this) {
>     int size = static_cast<int>(page->wasted_memory());
>     if (size == 0) {
>       // If there is no wasted memory then all free space is in the free list.
>       continue;
>     }
>     Address start = page->HighWaterMark();
>     Address end = page->area_end();
>     CHECK_EQ(size, static_cast<int>(end - start));
>     heap()->CreateFillerObjectAt(start, end, ClearRecordedSlots::kNo);
>   }

Done.

[ulan, 2017-04-06 15:52:53]

lgtm

[Slava Chigrin, 2017-04-06 16:12:49]

The CQ bit was checked by vchigrin@yandex-team.ru

[commit-bot: I haz the power, 2017-04-06 16:13:00]

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-04-06 16:42:30]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491495169594900,
"parent_rev": "639bfd2dc3a6a13d65c6baaf3bcd0e46fda757ff", "commit_rev":
"9ce9dde499792396a3997985f0867d5c92d7b18c"}

[commit-bot: I haz the power, 2017-04-06 16:42:36]

Description was changed from

==========
Fix free space repairing after deserialization.
==========

to

==========
Fix free space repairing after deserialization.

Review-Url: https://codereview.chromium.org/2806463002
Cr-Commit-Position: refs/heads/master@{#44455}
Committed:
https://chromium.googlesource.com/v8/v8/+/9ce9dde499792396a3997985f0867d5c92d...
==========

[commit-bot: I haz the power, 2017-04-06 16:42:37]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/9ce9dde499792396a3997985f0867d5c92d...