Chromium Code Reviews
Help | Chromium Project | Sign in
(21)

Issue 2799143002: Improve profile stats performace. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 weeks ago by brettw (plz ping after 24h)
Modified:
1 week, 1 day ago
Reviewers:
Roger Tawa
CC:
chromium-reviews, rginda+watch_chromium.org, oshima+watch_chromium.org, davemoore+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Improve profile stats performace. Collecting stats on prefs can take a lot of allocations and times to copy the whole preferences tree. Adds an iterator-with-callback API to read the preferences without copying. This is now used in prefs stats. The other two similar APIs in the preferences service have been coalesced with a flag controlling whether you want default prefs. BUG=708793 Review-Url: https://codereview.chromium.org/2799143002 Cr-Commit-Position: refs/heads/master@{#465374} Committed: https://chromium.googlesource.com/chromium/src/+/ebf7184f6f68e5b60a5247b65bb008a7b3ca1fb0

Patch Set 1 #

Patch Set 2 : Always #

Total comments: 2

Patch Set 3 : Use enum #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -73 lines) Patch
M chrome/browser/chromeos/preferences_chromeos_browsertest.cc View 1 2 1 chunk +10 lines, -6 lines 0 comments Download
M chrome/browser/feedback/system_logs/log_sources/chrome_internal_log_source.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/prefs/pref_functional_browsertest.cc View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M chrome/browser/profiles/profile_statistics_aggregator.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/profiles/profile_statistics_aggregator.cc View 2 chunks +15 lines, -19 lines 0 comments Download
M chrome/browser/ui/webui/local_state/local_state_ui.cc View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/prefs_internals_source.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M components/prefs/pref_service.h View 1 2 2 chunks +21 lines, -17 lines 0 comments Download
M components/prefs/pref_service.cc View 1 2 1 chunk +15 lines, -25 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 23 (15 generated)
brettw (plz ping after 24h)
Always
2 weeks, 3 days ago (2017-04-10 18:02:46 UTC) #7
brettw (plz ping after 24h)
Hi, you worked on the code review that added this here: https://codereview.chromium.org/1248613003
2 weeks, 3 days ago (2017-04-10 18:04:53 UTC) #9
brettw (plz ping after 24h)
For context I wrote this before I decided it would be better to delete the ...
2 weeks, 2 days ago (2017-04-11 19:53:08 UTC) #14
Roger Tawa
lgtm with one comment below. Thanks Brett. https://codereview.chromium.org/2799143002/diff/20001/components/prefs/pref_service.h File components/prefs/pref_service.h (right): https://codereview.chromium.org/2799143002/diff/20001/components/prefs/pref_service.h#newcode268 components/prefs/pref_service.h:268: bool include_defaults) ...
2 weeks, 1 day ago (2017-04-11 21:04:30 UTC) #15
brettw (plz ping after 24h)
https://codereview.chromium.org/2799143002/diff/20001/components/prefs/pref_service.h File components/prefs/pref_service.h (right): https://codereview.chromium.org/2799143002/diff/20001/components/prefs/pref_service.h#newcode268 components/prefs/pref_service.h:268: bool include_defaults) const; Done
1 week, 2 days ago (2017-04-18 19:28:39 UTC) #16
brettw (plz ping after 24h)
Use enum
1 week, 2 days ago (2017-04-18 19:28:43 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2799143002/40001
1 week, 2 days ago (2017-04-18 19:29:18 UTC) #20
commit-bot: I haz the power
1 week, 1 day ago (2017-04-18 21:11:08 UTC) #23
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/ebf7184f6f68e5b60a5247b65bb0...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46