New issue
Advanced search Search tips

Issue 651911 link

Starred by 1 user
Status: Fixed
Owner:
vakh@chromium.org
Closed: Feb 2017
Cc:
nparker@chromium.org
vakh@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 543161



Sign in to add a comment

Track and reduce the time it takes to perform PVer4 operations

Project Member Reported by vakh@chromium.org, Sep 30 2016 
This includes the time it takes to:
- Read from disk.
- Merge an update received from the server.
- Respond to the first resource reputation request.
- Respond to resource reputation request (median).
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 10 2016 

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

commit cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b
Author: vakh <vakh@chromium.org>
Date: Mon Oct 10 18:26:20 2016

PVer4: Test checksum on startup outside the hotpath of DB load

On startup, when MergeUpdate is called, the old_prefixes_map is empty. When we detect this case, we shortcut the merge process.
* Instead of merging the hash prefixes in lexicographical order, one by one, we copy the additions_map into the hash_prefix_map_.
* Then, instead of checking the checksum at this time, we post a task to the task runner to verify the checksum.
  When that task runs, if the checksum matches, it returns silently; otherwise, it clears out the state and the hash_prefix_map_ of the store.

This reduces the merge time for Malware and SocialEngineering lists by about 5 seconds total on my machine and brings the DB load time to <1s from 12s previously.

BUG= 651911 ,  543161 

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

[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_database.cc
[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_database.h
[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_database_unittest.cc
[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_local_database_manager.cc
[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_local_database_manager.h
[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_local_database_manager_unittest.cc
[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_store.cc
[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_store.h
[modify] https://crrev.com/cb8c22f7ca3eedb7e207149f1f75c90c3e6a748b/components/safe_browsing_db/v4_store_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 14 2016 

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

commit 9f1266e1e3cdb43dd9f5031f1dd3f5e7262a0d13
Author: vakh <vakh@chromium.org>
Date: Fri Oct 14 01:57:07 2016

Serialize the store's hash_prefix_map_ to file in WriteToDisk()

Associating with  bug 651911  since I am writing RAW hash prefixes on disk in this CL to improve startup time.

BUG= 543161 ,  651911 

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

[modify] https://crrev.com/9f1266e1e3cdb43dd9f5031f1dd3f5e7262a0d13/components/safe_browsing_db/v4_store.cc
[modify] https://crrev.com/9f1266e1e3cdb43dd9f5031f1dd3f5e7262a0d13/components/safe_browsing_db/v4_store.h
[modify] https://crrev.com/9f1266e1e3cdb43dd9f5031f1dd3f5e7262a0d13/components/safe_browsing_db/v4_store_unittest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 15 2016 

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

commit cfa3d9220463fcc9b19f3fe6c9c3a3a2a030b76e
Author: vakh <vakh@chromium.org>
Date: Sat Oct 15 01:44:00 2016

Delay checksum only on ReadFromDisk, not in FullUpdate cases
Existing code was delaying checksum check when the |old_prefixes_map|
was empty. However, this is also the case when we receive a FULL_UPDATE
response from the server. We call:
ProcessUpdate(HashPrefixMap(), response)
So checking for an empty |old_prefixes_map| is an incorrect condition.

This CL adds a flag: |delay_checksum_check| which is set to true only in
ReadFromDisk method.

BUG= 543161 ,  651911 

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

[modify] https://crrev.com/cfa3d9220463fcc9b19f3fe6c9c3a3a2a030b76e/components/safe_browsing_db/v4_store.cc
[modify] https://crrev.com/cfa3d9220463fcc9b19f3fe6c9c3a3a2a030b76e/components/safe_browsing_db/v4_store.h
[modify] https://crrev.com/cfa3d9220463fcc9b19f3fe6c9c3a3a2a030b76e/components/safe_browsing_db/v4_store_unittest.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 26 2016 

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

commit 73f60ce52a8b0abb4cd7612e1052f57cf316df56
Author: vakh <vakh@chromium.org>
Date: Wed Oct 26 01:09:37 2016

Remove incorrect ordering from V4 histograms for stores
Doing TBR since this has been discussed with holte@ over email.

TBR=holte

BUG= 651911 

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

[modify] https://crrev.com/73f60ce52a8b0abb4cd7612e1052f57cf316df56/tools/metrics/histograms/histograms.xml
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 5 2016 

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

commit 42c8810c0ab211a395596cd2f1b986097b0a90d7
Author: vakh <vakh@chromium.org>
Date: Sat Nov 05 01:32:49 2016

Update histogram names to follow the usual PVer4 format: SafeBrowsing.V4*

The new names also include the module that owns them, which makes it easier to
understand the histogram.

BUG= 651911 

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

[modify] https://crrev.com/42c8810c0ab211a395596cd2f1b986097b0a90d7/components/safe_browsing_db/v4_get_hash_protocol_manager.cc
[modify] https://crrev.com/42c8810c0ab211a395596cd2f1b986097b0a90d7/components/safe_browsing_db/v4_update_protocol_manager.cc
[modify] https://crrev.com/42c8810c0ab211a395596cd2f1b986097b0a90d7/tools/metrics/histograms/histograms.xml

Comment 9 by vakh@chromium.org, Nov 7 2016 

I think we now have all the histograms in place. I'm going to monitor them to see if we need to take any action to reduce the time taken for all the operations.

Comment 10 by vakh@chromium.org, Feb 6 2017

Status: Fixed (was: Assigned)
This is being tracked at: go/pver4-desktop-launch-metrics
►

Sign in to add a comment