New issue
Advanced search Search tips

Issue 775805 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Remove Uses of In-Place STLSetIntersection

Project Member Reported by mpear...@chromium.org, Oct 18 2017

Issue description


The only one I could find in the code base is
https://cs.chromium.org/chromium/src/components/bookmarks/browser/titled_url_index.cc

This is used in the omnibox code by BookmarksProvider.

Hence, it's a natural follow-up to be done by krb@
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 26 2017

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

commit 156ff6d76e1b9e7508f005da986eda27729224d2
Author: Kevin Bailey <krb@chromium.org>
Date: Thu Oct 26 17:36:00 2017

Add |IsNotIn| EraseIf-helper and convert to in-place set-intersection

There are a few places where the Omnibox and friends need set-intersection
but they currently construct a new set doing so. With EraseIf and this
IsNotIn helper, it's trivial to do in-place set-intersection. It showed a
20% improvement under micro-benchmarks.

This patch includes an existing Omnibox conversion, plus one under
components/bookmarks.

Bug:  775805 
Change-Id: Ic15b5d97c4c04ada4b1e70a3ad159d07909874ec
Reviewed-on: https://chromium-review.googlesource.com/726001
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Kevin Bailey <krb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511864}
[modify] https://crrev.com/156ff6d76e1b9e7508f005da986eda27729224d2/base/stl_util.h
[modify] https://crrev.com/156ff6d76e1b9e7508f005da986eda27729224d2/base/stl_util_unittest.cc
[modify] https://crrev.com/156ff6d76e1b9e7508f005da986eda27729224d2/components/bookmarks/browser/titled_url_index.cc
[modify] https://crrev.com/156ff6d76e1b9e7508f005da986eda27729224d2/components/omnibox/browser/url_index_private_data.cc

Comment 2 by k...@chromium.org, Oct 27 2017

Status: Fixed (was: Assigned)

Sign in to add a comment