New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 778468 link

Starred by 1 user
Status: Fixed
Owner:
█ hamelphi@chromium.org
Last visit > 30 days ago
Closed: Jan 2018
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Enforce predictor uniqueness through internal registration for AssistRanker

Project Member Reported by hamelphi@chromium.org, Oct 25 2017 
In the current state, Ranker does not enforce uniqueness of predictors. This could potentially lead to cache collisions if two predictors point to the same filename.

This could be solved by having a predictor registry in AssistRankerService. AssistRankerService would keep ownership of the predictors, and feature clients would request a pointer to a predictor using a unique identifier (which could be the filename).
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 15 2017 

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

commit c97535bd07eb551d7dd0669e675b481d8eb87f30
Author: Philippe Hamel <hamelphi@chromium.org>
Date: Fri Dec 15 19:15:40 2017

Ranker owns predictor objects and uses a config for initialization.

Ranker also takes care of logging and deals with field trials internally.

Bug: 786472,  778468 
Change-Id: Ie583616643ab8ad169df4739a4a18b81a950553f
Reviewed-on: https://chromium-review.googlesource.com/788331
Commit-Queue: Philippe Hamel <hamelphi@chromium.org>
Reviewed-by: Roger McFarlane <rogerm@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524426}
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLogger.java
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchRankerLoggerImpl.java
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchSelectionController.java
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/chrome/browser/android/chrome_feature_list.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/chrome/browser/android/chrome_feature_list.h
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/chrome/browser/android/contextualsearch/contextual_search_ranker_logger_impl.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/chrome/browser/android/contextualsearch/contextual_search_ranker_logger_impl.h
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/BUILD.gn
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/DEPS
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/assist_ranker_service.h
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/assist_ranker_service_impl.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/assist_ranker_service_impl.h
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/base_predictor.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/base_predictor.h
[add] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/base_predictor_unittest.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/binary_classifier_predictor.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/binary_classifier_predictor.h
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/binary_classifier_predictor_unittest.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/generic_logistic_regression_inference.cc
[add] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/predictor_config.cc
[add] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/predictor_config.h
[add] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/predictor_config_definitions.cc
[add] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/predictor_config_definitions.h
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/proto/ranker_example.proto
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/ranker_example_util.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/ranker_example_util.h
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/components/assist_ranker/ranker_example_util_unittest.cc
[modify] https://crrev.com/c97535bd07eb551d7dd0669e675b481d8eb87f30/services/metrics/public/cpp/ukm_recorder.h

Comment 2 by hamelphi@chromium.org, Jan 25 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment