New issue
Advanced search Search tips

Issue 923344 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Today
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

1.9%-2.1% regression in system_health.memory_desktop at 623790:623851

Project Member Reported by tdres...@chromium.org, Jan 18 (4 days ago)

Issue description

See the link to graphs below.
 
Project Member

Comment 1 by 42576172...@developer.gserviceaccount.com, Jan 18 (4 days ago)

All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=923344

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=8dd224872618daa6935d8794f1092fee49120e067264ac0f0ea5850752c65808


Bot(s) for this bug's original alert(s):

linux-perf

system_health.memory_desktop - Benchmark documentation link:
  https://bit.ly/system-health-benchmarks
Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Jan 18 (4 days ago)

Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jan 18 (4 days ago)

Cc: dtseng@chromium.org
Owner: dtseng@chromium.org
Status: Assigned (was: Unconfirmed)
📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/14b2d5d2540000

trigger event generation for reparented nodes on attribute changes by dtseng@chromium.org
https://chromium.googlesource.com/chromium/src/+/76429cf690c3b5811b7b7ee868f62b4956d3cbea
memory:chrome:all_processes:reported_by_chrome:malloc:effective_size: 1.101e+08 → 1.122e+08 (+2.085e+06)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

Benchmark documentation link:
  https://bit.ly/system-health-benchmarks
Project Member

Comment 4 by bugdroid1@chromium.org, Today (10 hours ago)

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

commit 6868807d31c58c60e90313c53c035232b2058137
Author: David Tseng <dtseng@chromium.org>
Date: Tue Jan 22 19:24:21 2019

Add AXNode::TakeData() to ensure proper move semantics

AXNode::data() returns a const AXNodeData& (lvalue), which triggers a copy constructor when called in the context of
std::map::insert.

The previous code triggered both a copy and move internal to std::map because it used AXNode::data().

In the current change, (by tracing all of the constructors of AXNodeData), there are now two move operations (via the move constructor) in AXNodeData when calling
std::map::insert on AXNode::TakeData().

This ensures no unnecessary copies are triggered.

Bug:  923344 
Change-Id: I53c7a8b8804e70fda93c44ee111162111287e268
Reviewed-on: https://chromium-review.googlesource.com/c/1423460
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624869}
[modify] https://crrev.com/6868807d31c58c60e90313c53c035232b2058137/ui/accessibility/ax_node.cc
[modify] https://crrev.com/6868807d31c58c60e90313c53c035232b2058137/ui/accessibility/ax_node.h
[modify] https://crrev.com/6868807d31c58c60e90313c53c035232b2058137/ui/accessibility/ax_tree.cc

Comment 5 by dtseng@chromium.org, Today (9 hours ago)

Status: Fixed (was: Assigned)

Sign in to add a comment