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

Issue 730032 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jun 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Potential incorrect cache management with translation events

Project Member Reported by etienneb@chromium.org, Jun 6 2017

Issue description


I ran a long-running test that was navigating to a restricted set of web pages for a whole day. The browser process was kept alive the whole time. Memory dumps using memory-infra are taken at the beginning and at the end of the experiment.

There are about 21582 objects that are potential leaks or potential incorrect cache management with the following stack-frame:

      "operator new",
      "google::protobuf::internal::ArenaStringPtr::CreateInstanceNoArena",
      "google::protobuf::internal::ArenaStringPtr::AssignWithDefault",
      "metrics::TranslateEventProto::MergeFrom",
      "metrics::TranslateEventProto::TranslateEventProto",
      "std::vector<...>::_Reallocate",
      "std::vector<...>::_Reserve",
      "std::vector<...>::push_back",
      "translate::TranslateRankerImpl::AddTranslateEvent",
      "translate::TranslateManager::InitiateTranslation",
      "translate::ContentTranslateDriver::RegisterPage",
      "translate::mojom::ContentTranslateDriverStubDispatch::Accept",
      "mojo::InterfaceEndpointClient::HandleValidatedMessage",
      "mojo::FilterChain::Accept",
      "mojo::internal::MultiplexRouter::ProcessIncomingMessage",
      "mojo::internal::MultiplexRouter::Accept",
      "mojo::FilterChain::Accept",
      "mojo::Connector::ReadSingleMessage",
      "mojo::Connector::ReadAllAvailableMessages",
      "mojo::SimpleWatcher::OnHandleReady",
      "base::internal::InvokeHelper<1,void>::MakeItSo<void (__cdecl subresource_filter::SubframeNavigationFilteringThrottle::*const & __ptr64)(enum subresource_filter::SubframeNavigationFilteringThrottle::ThrottlingStage,enum subresource_filter::LoadPolicy) __ptr64,base::WeakPtr<subresource_filter::SubframeNavigationFilteringThrottle> const & __ptr64,enum subresource_filter::SubframeNavigationFilteringThrottle::ThrottlingStage const & __ptr64,enum subresource_filter::LoadPolicy>",
      "base::debug::TaskAnnotator::RunTask",
      "base::MessageLoop::RunTask",
      "base::MessageLoop::DoWork",
      "base::MessagePumpForUI::DoRunLoop",
      "base::MessagePumpWin::Run",
      "base::RunLoop::Run",
      "ChromeBrowserMainParts::MainMessageLoopRun",
      "content::BrowserMainLoop::RunMainMessageLoopParts",
      "content::BrowserMainRunnerImpl::Run",
      "content::BrowserMain",
      "content::RunNamedProcessTypeMain",
      "content::ContentMainRunnerImpl::Run",
      "service_manager::Main",
      "content::ContentMain",
      "ChromeMain",
      "MainDllLoader::Launch",
      "wWinMain",
      "__scrt_common_main_seh",
      "BaseThreadInitThunk",
      "RtlUserThreadStart",
      "[Thread: CrBrowserMain]"

The following line is adding the events:
  event_cache_.push_back(event);

  https://cs.chromium.org/chromium/src/components/translate/core/browser/translate_ranker_impl.cc?l=327&rcl=0f8c781859c2e2767a543d0238870c105a0ece88

They are cleared with:
  TranslateRankerImpl::FlushTranslateEvents(

which are called periodically.

Is is possible that these objects are never flushed under my experiment conditions?

The experiment was done with the following patch over telemetry:
  https://codereview.chromium.org/2906113002/



 
Owner: hamelphi@chromium.org
Status: Started (was: Untriaged)
A solution to this would be to listen for Disable/Enable recording in the code that accumulates the proto.
Cc: -alexis@chromium.org asvitk...@chromium.org
Potentially what I wrote in  crbug.com/730091  might be relevant to this one too.
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 12 2017

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

commit a33125bccd08d77590b3ee8907d85ca5a042c78a
Author: hamelphi <hamelphi@chromium.org>
Date: Mon Jun 12 19:40:57 2017

Send UMA recording state to TranslateRanker.

This will prevent the accumulation of translate events when UMA recording is not enabled.

Also:
 - clear the event_cache when recording is disabled.
 - Change 'return' to 'continue' in translate_ranker_metrics_provider in case only a subset of browser states don't have a valid ranker.

BUG= 730032 

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

[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/chrome/browser/translate/translate_ranker_metrics_provider.cc
[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/chrome/browser/translate/translate_ranker_metrics_provider.h
[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/components/translate/core/browser/mock_translate_ranker.h
[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/components/translate/core/browser/translate_ranker.h
[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/components/translate/core/browser/translate_ranker_impl.cc
[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/components/translate/core/browser/translate_ranker_impl.h
[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/components/translate/core/browser/translate_ranker_impl_unittest.cc
[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/ios/chrome/browser/translate/translate_ranker_metrics_provider.cc
[modify] https://crrev.com/a33125bccd08d77590b3ee8907d85ca5a042c78a/ios/chrome/browser/translate/translate_ranker_metrics_provider.h

Status: Fixed (was: Started)

Sign in to add a comment