Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2763863002:
[USS] Remove intermediate structure during GetAllSyncMetadata (Closed)
|
Created:
3 years, 9 months ago by Gang Wu Modified:
3 years, 9 months ago CC:
chromium-reviews, rouslan+autofill_chromium.org, rogerm+autofillwatch_chromium.org, sebsg+autofillwatch_chromium.org, browser-components-watch_chromium.org, mathp+autofillwatch_chromium.org, vabr+watchlistautofill_chromium.org, estade+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[USS] Remove intermediate structure during GetAllSyncMetadata to reduce memory
usage
BUG=701670
Review-Url: https://codereview.chromium.org/2763863002
Cr-Commit-Position: refs/heads/master@{#459255}
Committed: https://chromium.googlesource.com/chromium/src/+/d6b104cb86ef80a415014191e2f10b6e0dbd501b
Patch Set 1 #
Total comments: 2
Patch Set 2 : Using DLOG(WARNING) #
Messages
Total messages: 31 (22 generated)
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
Description was changed from ========== save memory BUG= ========== to ========== [USS] Remove intermediate structure during GetAllSyncMetadata to reduce memory usage BUG=701670 ==========
gangwu@chromium.org changed reviewers: + pavely@chromium.org
PTAL
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm % one comment. https://codereview.chromium.org/2763863002/diff/20001/components/history/core... File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2763863002/diff/20001/components/history/core... components/history/core/browser/typed_url_sync_metadata_database.cc:117: VLOG(0) << "Failed to deserialize TYPED_URLS model type " VLOG(0) is not really used much in chromium code because - it is not compiled out in retail builds like DVLOG does - It is not enabled by default like LOG(WARNING). Additional steps are required to collect these logs from customer device. You essentially pay the cost of having it in customers' binaries without much benefit. Think about in which scenarios you would want to see this message and pick appropriate logging macro. I'd use DLOG(WARNING) here. The same comment applies for the other location.
gangwu@chromium.org changed reviewers: + brettw@chromium.org, mathp@chromium.org
Hi, mathp@, please review for components/autofill/core/browser/webdata/autofill_table.cc components/autofill/core/browser/webdata/autofill_table.h brettw, please review for components/history/core/browser/typed_url_sync_metadata_database.cc components/history/core/browser/typed_url_sync_metadata_database.h https://codereview.chromium.org/2763863002/diff/20001/components/history/core... File components/history/core/browser/typed_url_sync_metadata_database.cc (right): https://codereview.chromium.org/2763863002/diff/20001/components/history/core... components/history/core/browser/typed_url_sync_metadata_database.cc:117: VLOG(0) << "Failed to deserialize TYPED_URLS model type " On 2017/03/21 19:10:47, pavely wrote: > VLOG(0) is not really used much in chromium code because > - it is not compiled out in retail builds like DVLOG does > - It is not enabled by default like LOG(WARNING). Additional steps are required > to collect these logs from customer device. > You essentially pay the cost of having it in customers' binaries without much > benefit. > > Think about in which scenarios you would want to see this message and pick > appropriate logging macro. I'd use DLOG(WARNING) here. > > The same comment applies for the other location. Done.
The CQ bit was checked by gangwu@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM, thanks for following up quickly.
autofill lgtm
The CQ bit was checked by gangwu@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pavely@chromium.org Link to the patchset: https://codereview.chromium.org/2763863002/#ps40001 (title: "Using DLOG(WARNING)")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by gangwu@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1490307021817630,
"parent_rev": "331224fb38ef3a2ef7bcc5eadb7592a0bebd0d02", "commit_rev":
"d6b104cb86ef80a415014191e2f10b6e0dbd501b"}
Message was sent while issue was closed.
Description was changed from ========== [USS] Remove intermediate structure during GetAllSyncMetadata to reduce memory usage BUG=701670 ========== to ========== [USS] Remove intermediate structure during GetAllSyncMetadata to reduce memory usage BUG=701670 Review-Url: https://codereview.chromium.org/2763863002 Cr-Commit-Position: refs/heads/master@{#459255} Committed: https://chromium.googlesource.com/chromium/src/+/d6b104cb86ef80a415014191e2f1... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/chromium/src/+/d6b104cb86ef80a415014191e2f1... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
