New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 637217 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 632021



Sign in to add a comment

Refactor octane.py to use GetSystemTotalPhysicalMemory directly

Project Member Reported by perezju@chromium.org, Aug 12 2016

Issue description

The octane Telemetry benchmark [1] today uses browser.memory_stats to get the total amount of physical memory available on the platform.

Since browser.memory_stats is being deprecated, it should be refactored to call platform.GetSystemTotalPhysicalMemory directly instead.

[1]: https://cs.chromium.org/chromium/src/tools/perf/benchmarks/octane.py?l=83
 
Blocking: 632021
I am not even sure that we need to track memory usage for this hypothetical benchmark. If we want to keep "non-tracing" memory data for comparing between like Chrome 45 & 55, probably better to use the system health stories? 
Note, from the code, this benchmark is *not* tracking memory; it only uses platform.GetSystemTotalPhysicalMemory (which, for each platform, should be a constant), to determine whether to skip some specific micro-benchmark.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62f4b56e57376b13a7bab82e50ca03c2d5033e63

commit 62f4b56e57376b13a7bab82e50ca03c2d5033e63
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Sat Sep 03 23:07:49 2016

Roll src/third_party/catapult/ 4f812c903..1f08948ad (1 commit).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/4f812c9035e6..1f08948ad3a9

$ git log 4f812c903..1f08948ad --date=short --no-merges --format='%ad %ae %s'
2016-09-03 perezju [Telemetry] Expose platform.GetSystemTotalPhysicalMemory

BUG= 637217 

TBR=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2311593002
Cr-Commit-Position: refs/heads/master@{#416456}

[modify] https://crrev.com/62f4b56e57376b13a7bab82e50ca03c2d5033e63/DEPS

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 3 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/233fa6caaa0206814af3213a04ffe27c914991ec

commit 233fa6caaa0206814af3213a04ffe27c914991ec
Author: dominicc <dominicc@chromium.org>
Date: Sat Sep 03 23:25:20 2016

Revert of Roll src/third_party/catapult/ 4f812c903..1f08948ad (1 commit). (patchset #1 id:1 of https://codereview.chromium.org/2311593002/ )

Reason for revert:
This appears to have broken the Google Chrome Win builder.

Original issue's description:
> Roll src/third_party/catapult/ 4f812c903..1f08948ad (1 commit).
>
> https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/4f812c9035e6..1f08948ad3a9
>
> $ git log 4f812c903..1f08948ad --date=short --no-merges --format='%ad %ae %s'
> 2016-09-03 perezju [Telemetry] Expose platform.GetSystemTotalPhysicalMemory
>
> BUG= 637217 
>
> TBR=catapult-sheriff@chromium.org
>
> Committed: https://crrev.com/62f4b56e57376b13a7bab82e50ca03c2d5033e63
> Cr-Commit-Position: refs/heads/master@{#416456}

TBR=catapult-sheriff@chromium.org,catapult-deps-roller@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 637217 

Review-Url: https://codereview.chromium.org/2310733002
Cr-Commit-Position: refs/heads/master@{#416457}

[modify] https://crrev.com/233fa6caaa0206814af3213a04ffe27c914991ec/DEPS

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aeaf20d92a030f2922389b0ed3cc920f19a82332

commit aeaf20d92a030f2922389b0ed3cc920f19a82332
Author: catapult-deps-roller <catapult-deps-roller@chromium.org>
Date: Sun Sep 04 00:39:15 2016

Roll src/third_party/catapult/ 4f812c903..1f08948ad (1 commit).

https://chromium.googlesource.com/external/github.com/catapult-project/catapult.git/+log/4f812c9035e6..1f08948ad3a9

$ git log 4f812c903..1f08948ad --date=short --no-merges --format='%ad %ae %s'
2016-09-03 perezju [Telemetry] Expose platform.GetSystemTotalPhysicalMemory

BUG= 637217 

TBR=catapult-sheriff@chromium.org

Review-Url: https://codereview.chromium.org/2309063002
Cr-Commit-Position: refs/heads/master@{#416459}

[modify] https://crrev.com/aeaf20d92a030f2922389b0ed3cc920f19a82332/DEPS

Cc: sullivan@chromium.org dominicc@chromium.org
Looks like dominicc reverted the roll, but the auto-roller just went ahead and re-rolled the CL again. Anyway, the CL is now in, and the Google Chrome Win builder is green. So I suspect the original failure was just a fluke.
I think so. I don't think telemetry test has anything to do with Google Chrome Win builder
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0557c1fcaafb27f8cdfcfd8f0717fc22e5388bcd

commit 0557c1fcaafb27f8cdfcfd8f0717fc22e5388bcd
Author: perezju <perezju@chromium.org>
Date: Mon Sep 05 10:38:51 2016

[tools/perf] Refactor octane benchmark to use GetSystemTotalPhysicalMemory

The browser.memory_stats property is being deprecated, so we switch to
request platform.GetSystemTotalPhysicalMemory directly.

Depends on CL:
https://codereview.chromium.org/2308633002

BUG= 637217 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:android_s5_perf_cq;master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq

Review-Url: https://codereview.chromium.org/2310433002
Cr-Commit-Position: refs/heads/master@{#416522}

[modify] https://crrev.com/0557c1fcaafb27f8cdfcfd8f0717fc22e5388bcd/tools/perf/benchmarks/octane.py

Status: Fixed (was: Assigned)

Sign in to add a comment