[heap] Base number of compaction tasks on live memory and compaction speed.

[heap] Base number of compaction tasks on live memory and compaction speed.

BUG=chromium:524425
LOG=N

Committed: https://crrev.com/5b89001ccd7746827469ee3966f573842cdf73d0
Cr-Commit-Position: refs/heads/master@{#31762}

[Michael Lippautz, 2015-11-03 03:48:03]

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

[commit-bot: I haz the power, 2015-11-03 03:48:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410633005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410633005/20001

[commit-bot: I haz the power, 2015-11-03 04:05:00]

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

[commit-bot: I haz the power, 2015-11-03 04:05:01]

Dry run: This issue passed the CQ dry run.

[Hannes Payer (out of office), 2015-11-03 04:08:17]

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

[Hannes Payer (out of office), 2015-11-03 04:08:17]

https://codereview.chromium.org/1410633005/diff/20001/src/heap/gc-tracer.h
File src/heap/gc-tracer.h (right):

https://codereview.chromium.org/1410633005/diff/20001/src/heap/gc-tracer.h#ne...
src/heap/gc-tracer.h:425: intptr_t CompactionSpeedInBytesPerMillisecond() const;
Add comment, like others.

https://codereview.chromium.org/1410633005/diff/20001/src/heap/gc-tracer.h#ne...
src/heap/gc-tracer.h:541: CompactionEventBuffer compaction_events_;
Comment.

https://codereview.chromium.org/1410633005/diff/20001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/1410633005/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3316: const double kTargetCompactionTimeInMs = 2;
Let's aim for 1 ms. Keep the constant, it has at least a documentation purpose.

https://codereview.chromium.org/1410633005/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3466: intptr_t live_bytes = p->LiveBytes();
Let's move the time measuring into EvacuateLiveObjectsFromPage.

[Michael Lippautz, 2015-11-03 20:57:42]

PTAL. I restructured how the number of tasks is computed. It should be way
easier to follow now.

https://codereview.chromium.org/1410633005/diff/20001/src/heap/gc-tracer.h
File src/heap/gc-tracer.h (right):

https://codereview.chromium.org/1410633005/diff/20001/src/heap/gc-tracer.h#ne...
src/heap/gc-tracer.h:425: intptr_t CompactionSpeedInBytesPerMillisecond() const;
On 2015/11/03 04:08:17, Hannes Payer wrote:
> Add comment, like others.

Done.

https://codereview.chromium.org/1410633005/diff/20001/src/heap/gc-tracer.h#ne...
src/heap/gc-tracer.h:541: CompactionEventBuffer compaction_events_;
On 2015/11/03 04:08:17, Hannes Payer wrote:
> Comment.

Done.

https://codereview.chromium.org/1410633005/diff/20001/src/heap/mark-compact.cc
File src/heap/mark-compact.cc (right):

https://codereview.chromium.org/1410633005/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3316: const double kTargetCompactionTimeInMs = 2;
On 2015/11/03 04:08:17, Hannes Payer wrote:
> Let's aim for 1 ms. Keep the constant, it has at least a documentation
purpose.

Done.

I also restructured the bottom part (return statement) and the order of caps in
the comments now matches the code.

https://codereview.chromium.org/1410633005/diff/20001/src/heap/mark-compact.c...
src/heap/mark-compact.cc:3466: intptr_t live_bytes = p->LiveBytes();
On 2015/11/03 04:08:17, Hannes Payer wrote:
> Let's move the time measuring into EvacuateLiveObjectsFromPage.

Done.

[Hannes Payer (out of office), 2015-11-03 21:19:32]

lgtm

[Michael Lippautz, 2015-11-03 21:23:29]

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

[commit-bot: I haz the power, 2015-11-03 21:23:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410633005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410633005/60001

[commit-bot: I haz the power, 2015-11-03 21:41:28]

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

[commit-bot: I haz the power, 2015-11-03 21:41:29]

Dry run: This issue passed the CQ dry run.

[Michael Lippautz, 2015-11-03 21:42:37]

The CQ bit was checked by mlippautz@chromium.org

[commit-bot: I haz the power, 2015-11-03 21:42:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410633005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410633005/60001

[commit-bot: I haz the power, 2015-11-03 21:44:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-03 21:44:13]

Patchset 4 (id:??) landed as
https://crrev.com/5b89001ccd7746827469ee3966f573842cdf73d0
Cr-Commit-Position: refs/heads/master@{#31762}

[Lei Zhang, 2015-11-12 01:14:36]

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

[Lei Zhang, 2015-11-12 01:14:36]

Hi, we have a copy of v8 in standalone PDFium builds. I tried to roll DEPS past
this CL and got a failure on Windows:

http://build.chromium.org/p/client.pdfium/builders/windows/builds/105

FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft
Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC
@obj\v8\src\heap\v8_base_0.gc-tracer.obj.rsp /c ..\..\v8\src\heap\gc-tracer.cc
/Foobj\v8\src\heap\v8_base_0.gc-tracer.obj /Fdobj\v8\tools\gyp\v8_base_0.cc.pdb 

e:\b\build\slave\windows\build\pdfium\v8\src\heap\gc-tracer.cc(721) : error
C2220: warning treated as error - no 'object' file generated

e:\b\build\slave\windows\build\pdfium\v8\src\heap\gc-tracer.cc(721) : warning
C4244: 'return' : conversion from 'double' to 'intptr_t', possible loss of data

[Michael Lippautz, 2015-11-12 19:17:55]

On 2015/11/12 01:14:36, Lei Zhang wrote:
> Hi, we have a copy of v8 in standalone PDFium builds. I tried to roll DEPS
past
> this CL and got a failure on Windows:
> 
> http://build.chromium.org/p/client.pdfium/builders/windows/builds/105
> 
> FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft
> Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC
> @obj\v8\src\heap\v8_base_0.gc-tracer.obj.rsp /c ..\..\v8\src\heap\gc-tracer.cc
> /Foobj\v8\src\heap\v8_base_0.gc-tracer.obj
/Fdobj\v8\tools\gyp\v8_base_0.cc.pdb 
> 
> e:\b\build\slave\windows\build\pdfium\v8\src\heap\gc-tracer.cc(721) : error
> C2220: warning treated as error - no 'object' file generated
> 
> e:\b\build\slave\windows\build\pdfium\v8\src\heap\gc-tracer.cc(721) : warning
> C4244: 'return' : conversion from 'double' to 'intptr_t', possible loss of
data

Fix is on the way:
  https://codereview.chromium.org/1438313002/

[Lei Zhang, 2015-11-12 19:39:48]

On 2015/11/12 19:17:55, Michael Lippautz wrote:
> Fix is on the way:
>   https://codereview.chromium.org/1438313002/

Thank you!