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

Issue 910251 link

Starred by 1 user
Status: Fixed
Owner:
jegray@chromium.org
Closed: Dec 27
Cc:
tbansal@chromium.org
█ nyquist@chromium.org
ryansturm@chromium.org
dougarnett@chromium.org
bengr@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Potential jank caused by OptimizationGuideService / PreviewsOptimizationGuide at startup

Project Member Reported by ssid@chromium.org, Nov 29 
We collected slow reports from users facing janks and found that the function mentioned above is taking too much time on the main thread, potentially causing janks.
The issue can be found in reports:

OptimizationGuideService::DispatchHintsOnUIThread()
ID                Duration ms
ce57b4a159cd7e90 ,377.018
b68e08be045d64be ,324.87
1c26b44bdb641e0e ,256.42
19cd65a323f45fcf ,243.481
1a08a331bf9b3dde ,229.222
e3458113db83303f ,209.215

PreviewsOptimizationGuide::UpdateHints()
4d8d475742f93df9 ,279.101
24f58f49c2d75ab3 ,279.101
8cd6b5dc11e9fd6c ,298.196
b9779bf5a8ca9524 ,246.582
b68e08be045d64be ,163.455
19cd65a323f45fcf ,147.603


Go to crash/ReportID to view the traces.

The stack trace that was hit the most:
The stacks are not very clean because of scanning. If you need better stack quality for the reports we can try to find the reports with good stack.

nonymous namespace)::RealFree(base::allocator::AllocatorDispatch const*, void*, void*)
std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::~basic_string()
google::protobuf::internal::ArenaStringPtr::DestroyNoArena(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const*)
optimization_guide::proto::ResourceLoadingHint::~ResourceLoadingHint()
optimization_guide::proto::ResourceLoadingHint::~ResourceLoadingHint()
optimization_guide::proto::Optimization::~Optimization()
(anonymous namespace)::RealFree(base::allocator::AllocatorDispatch const*, void*, void*)
optimization_guide::proto::Optimization::~Optimization()
void google::protobuf::internal::RepeatedPtrFieldBase::Destroy<google::protobuf::RepeatedPtrField<google::protobuf::MessageLite>::TypeHandler>()
(anonymous namespace)::RealFree(base::allocator::AllocatorDispatch const*, void*, void*)
optimization_guide::proto::PageHint::~PageHint()
optimization_guide::proto::PageHint::~PageHint()
optimization_guide::proto::Hint::~Hint()
previews::HintCache::AddHints(std::__ndk1::vector<optimization_guide::proto::Hint, std::__ndk1::allocator<optimization_guide::proto::Hint> > const&)
previews::PreviewsOptimizationGuide::UpdateHints(std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> >)
previews::PreviewsHints::Initialize()
base::internal::Invoker<base::internal::BindState<void (previews::PreviewsOptimizationGuide::*)(std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> >), base::WeakPtr<previews::PreviewsOptimizationGuide> >, void (std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> >)>::RunOnce(base::internal::BindStateBase*, std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> >&&)
base::(anonymous namespace)::PostTaskAndReplyRelay::RunReply(base::(anonymous namespace)::PostTaskAndReplyRelay)
void base::internal::ReplyAdapter<std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> >, std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> > >(base::OnceCallback<void (std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> >)>, std::__ndk1::unique_ptr<std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> >, std::__ndk1::default_delete<std::__ndk1::unique_ptr<previews::PreviewsHints, std::__ndk1::default_delete<previews::PreviewsHints> > > >*)
base::internal::Invoker<base::internal::BindState<void (*)(base::OnceCallback<void (std::__ndk1::unique_ptr<policy::URLBlacklist, std::__ndk1::default_delete<policy::URLBlacklist> >)>, std::__ndk1::unique_ptr<std::__ndk1::unique_ptr<policy::URLBlacklist, std::__ndk1::default_delete<policy::URLBlacklist> >, std::__ndk1::default_delete<std::__ndk1::unique_ptr<policy::URLBlacklist, std::__ndk1::default_delete<policy::URLBlacklist> > > >*), base::OnceCallback<void (std::__ndk1::unique_ptr<policy::URLBlacklist, std::__ndk1::default_delete<policy::URLBlacklist> >)>, base::internal::OwnedWrapper<std::__ndk1::unique_ptr<std::__ndk1::unique_ptr<policy::URLBlacklist, std::__ndk1::default_delete<policy::URLBlacklist> >, std::__ndk1::default_delete<std::__ndk1::unique_ptr<policy::URLBlacklist, std::__ndk1::default_delete<policy::URLBlacklist> > > > > >, void ()>::RunOnce(base::internal::BindStateBase*)
base::(anonymous namespace)::PostTaskAndReplyRelay::RunReply(base::(anonymous namespace)::PostTaskAndReplyRelay)
void base::internal::FunctorTraits<void (*)(base::(anonymous namespace)::PostTaskAndReplyRelay), void>::Invoke<void (*)(base::(anonymous namespace)::PostTaskAndReplyRelay), base::(anonymous namespace)::PostTaskAndReplyRelay>(void (*&&)(base::(anonymous namespace)::PostTaskAndReplyRelay), base::(anonymous namespace)::PostTaskAndReplyRelay&&)
previews::PreviewsOptimizationGuide::OnHintsProcessed(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
previews::PreviewsOptimizationGuide::OnHintsProcessed(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
previews::PreviewsOptimizationGuide::OnHintsProcessed(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
optimization_guide::OptimizationGuideService::ProcessHintsInBackground(optimization_guide::ComponentInfo const&)
optimization_guide::OptimizationGuideService::ProcessHints(optimization_guide::ComponentInfo const&)
component_updater::ComponentInstaller::Register(component_updater::ComponentUpdateService*, base::OnceCallback<void ()>)
base::MessageLoopImpl::DoWork()


mojo::core::RequestContext::~RequestContext()
           base::internal::BindStateBase::BindStateBase(void (*)(), void (*)(base::internal::BindStateBase const*))
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           operator new(unsigned int)
           std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::__init(char const*, unsigned int)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::basic_string(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&)
           google::protobuf::internal::ArenaStringPtr::CreateInstanceNoArena(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const*)
           quic::QuicServerConfigProtobuf_PrivateKey::MergeFrom(quic::QuicServerConfigProtobuf_PrivateKey const&)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::ResourceLoadingHint>::TypeHandler>(void**, void**, int, int)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::ResourceLoadingHint>::TypeHandler>(void**, void**, int, int)
           google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInternal(google::protobuf::internal::RepeatedPtrFieldBase const&, void (google::protobuf::internal::RepeatedPtrFieldBase::*)(void**, void**, int, int))
           optimization_guide::proto::Optimization::MergeFrom(optimization_guide::proto::Optimization const&)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::Optimization>::TypeHandler>(void**, void**, int, int)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::Optimization>::TypeHandler>(void**, void**, int, int)
           google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInternal(google::protobuf::internal::RepeatedPtrFieldBase const&, void (google::protobuf::internal::RepeatedPtrFieldBase::*)(void**, void**, int, int))
           optimization_guide::proto::PageHint::MergeFrom(optimization_guide::proto::PageHint const&)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::PageHint>::TypeHandler>(void**, void**, int, int)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::PageHint>::TypeHandler>(void**, void**, int, int)
           google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInternal(google::protobuf::internal::RepeatedPtrFieldBase const&, void (google::protobuf::internal::RepeatedPtrFieldBase::*)(void**, void**, int, int))
           optimization_guide::proto::Hint::MergeFrom(optimization_guide::proto::Hint const&)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::Hint>::TypeHandler>(void**, void**, int, int)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::Hint>::TypeHandler>(void**, void**, int, int)
           google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInternal(google::protobuf::internal::RepeatedPtrFieldBase const&, void (google::protobuf::internal::RepeatedPtrFieldBase::*)(void**, void**, int, int))
           operator new(unsigned int)
           optimization_guide::proto::Configuration::Configuration(optimization_guide::proto::Configuration const&)
           base::internal::BindStateBase::BindStateBase(void (*)(), void (*)(base::internal::BindStateBase const*))
           previews::PreviewsOptimizationGuide::OnHintsProcessed(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           previews::PreviewsHints::CreateFromConfig(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           previews::PreviewsOptimizationGuide::OnHintsProcessed(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           optimization_guide::OptimizationGuideService::DispatchHintsOnUIThread(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           base::internal::Invoker<base::internal::BindState<void (optimization_guide::OptimizationGuideService::*)(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&), base::internal::UnretainedWrapper<optimization_guide::OptimizationGuideService>, optimization_guide::proto::Configuration, optimization_guide::ComponentInfo>, void ()>::RunOnce(base::internal::BindStateBase*)
           base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
           optimization_guide::OptimizationGuideService::ProcessHintsInBackground(optimization_guide::ComponentInfo const&)
           optimization_guide::OptimizationGuideService::ProcessHints(optimization_guide::ComponentInfo const&)
           component_updater::ComponentInstaller::Register(component_updater::ComponentUpdateService*, base::OnceCallback<void ()>)
           component_updater::ComponentInstaller::Register(component_updater::ComponentUpdateService*, base::OnceCallback<void ()>)
           content::StartupTaskRunner::WrappedTask()
           base::MessageLoopImpl::DoWork()
           optimization_guide::OptimizationGuideService::ProcessHintsInBackground(optimization_guide::ComponentInfo const&)
           base::internal::PendingTaskQueue::DelayedQueue::HasTasks()
           optimization_guide::OptimizationGuideService::ProcessHintsInBackground(optimization_guide::ComponentInfo const&)
           std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::__grow_by(unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int)






           std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::__init(char const*, unsigned int)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::basic_string(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&)
           google::protobuf::internal::ArenaStringPtr::CreateInstanceNoArena(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const*)
           quic::QuicServerConfigProtobuf_PrivateKey::MergeFrom(quic::QuicServerConfigProtobuf_PrivateKey const&)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::ResourceLoadingHint>::TypeHandler>(void**, void**, int, int)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::ResourceLoadingHint>::TypeHandler>(void**, void**, int, int)
           google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInternal(google::protobuf::internal::RepeatedPtrFieldBase const&, void (google::protobuf::internal::RepeatedPtrFieldBase::*)(void**, void**, int, int))
           optimization_guide::proto::Optimization::MergeFrom(optimization_guide::proto::Optimization const&)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::Optimization>::TypeHandler>(void**, void**, int, int)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::Optimization>::TypeHandler>(void**, void**, int, int)
           google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInternal(google::protobuf::internal::RepeatedPtrFieldBase const&, void (google::protobuf::internal::RepeatedPtrFieldBase::*)(void**, void**, int, int))
           optimization_guide::proto::PageHint::MergeFrom(optimization_guide::proto::PageHint const&)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::PageHint>::TypeHandler>(void**, void**, int, int)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::PageHint>::TypeHandler>(void**, void**, int, int)
           google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInternal(google::protobuf::internal::RepeatedPtrFieldBase const&, void (google::protobuf::internal::RepeatedPtrFieldBase::*)(void**, void**, int, int))
           optimization_guide::proto::Hint::MergeFrom(optimization_guide::proto::Hint const&)
           (anonymous namespace)::RealMalloc(base::allocator::AllocatorDispatch const*, unsigned int, void*)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::Hint>::TypeHandler>(void**, void**, int, int)
           void google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInnerLoop<google::protobuf::RepeatedPtrField<optimization_guide::proto::Hint>::TypeHandler>(void**, void**, int, int)
           google::protobuf::internal::RepeatedPtrFieldBase::MergeFromInternal(google::protobuf::internal::RepeatedPtrFieldBase const&, void (google::protobuf::internal::RepeatedPtrFieldBase::*)(void**, void**, int, int))
           operator new(unsigned int)
           optimization_guide::proto::Configuration::Configuration(optimization_guide::proto::Configuration const&)
           base::internal::BindStateBase::BindStateBase(void (*)(), void (*)(base::internal::BindStateBase const*))
           previews::PreviewsOptimizationGuide::OnHintsProcessed(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           previews::PreviewsHints::CreateFromConfig(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           previews::PreviewsOptimizationGuide::OnHintsProcessed(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           optimization_guide::OptimizationGuideService::DispatchHintsOnUIThread(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           base::internal::Invoker<base::internal::BindState<void (optimization_guide::OptimizationGuideService::*)(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&), base::internal::UnretainedWrapper<optimization_guide::OptimizationGuideService>, optimization_guide::proto::Configuration, optimization_guide::ComponentInfo>, void ()>::RunOnce(base::internal::BindStateBase*)
           base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
           optimization_guide::OptimizationGuideService::ProcessHintsInBackground(optimization_guide::ComponentInfo const&)
           optimization_guide::OptimizationGuideService::ProcessHints(optimization_guide::ComponentInfo const&)
           component_updater::ComponentInstaller::Register(component_updater::ComponentUpdateService*, base::OnceCallback<void ()>)
           component_updater::ComponentInstaller::Register(component_updater::ComponentUpdateService*, base::OnceCallback<void ()>)
           content::StartupTaskRunner::WrappedTask()
           base::MessageLoopImpl::DoWork()
           optimization_guide::OptimizationGuideService::ProcessHintsInBackground(optimization_guide::ComponentInfo const&)
           base::internal::PendingTaskQueue::DelayedQueue::HasTasks()
           optimization_guide::OptimizationGuideService::ProcessHintsInBackground(optimization_guide::ComponentInfo const&)
           std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> >::__grow_by(unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int)
           optimization_guide::OptimizationGuideService::ProcessHints(optimization_guide::ComponentInfo const&)
           component_updater::ComponentInstaller::Register(component_updater::ComponentUpdateService*, base::OnceCallback<void ()>)
           component_updater::ComponentInstaller::Register(component_updater::ComponentUpdateService*, base::OnceCallback<void ()>)
           content::StartupTaskRunner::WrappedTask()



 __ThumbV7PILongThunk__ZN2v88internal21PerIsolateAssertScopeILNS0_20PerIsolateAssertTypeE2ELb0EE9IsAllowedEPNS0_7IsolateE
           optimization_guide::ComponentInfo::ComponentInfo(optimization_guide::ComponentInfo const&)
           previews::PreviewsOptimizationGuide::OnHintsProcessed(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           optimization_guide::OptimizationGuideService::DispatchHintsOnUIThread(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&)
           base::internal::Invoker<base::internal::BindState<void (optimization_guide::OptimizationGuideService::*)(optimization_guide::proto::Configuration const&, optimization_guide::ComponentInfo const&), base::internal::UnretainedWrapper<optimization_guide::OptimizationGuideService>, optimization_guide::proto::Configuration, optimization_guide::ComponentInfo>, void ()>::RunOnce(base::internal::BindStateBase*)
           base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
           base::MessageLoopImpl::DoWork()
           base::MessagePumpForUI::OnNonDelayedLooperCallback()
           base::(anonymous namespace)::NonDelayedLooperCallback(int, int, void*)

Comment 1 by ssid@chromium.org, Nov 29

Labels: Performance-Startup
Summary:
OptimizationGuideService::DispatchHintsOnUIThread() - affecting > 8% of users with 100 ms janks

PreviewsOptimizationGuide::UpdateHints() - affecting >2.5% of users with 100 ms jank.

Find some stack traces above from samples which are probable cause of the issue.

Comment 2 by ssid@chromium.org, Nov 29

Cc: nyquist@chromium.org ryansturm@chromium.org dougarnett@chromium.org rajendrant@chromium.org tbansal@chromium.org
Components: Blink>Previews Internals>Network>DataProxy
Owner: tbansal@chromium.org
These have started showing up lot more in the recent 2 weeks, and affecting a lot of users.

Comment 3 by ssid@chromium.org, Nov 29

Summary: Potential jank caused by OptimizationGuideService / PreviewsOptimizationGuide at startup (was: Potential jank caused by OptimizationGuideService / PreviewsOptimizationGuide)

Comment 4 by tbansal@chromium.org, Nov 29

Cc: -rajendrant@chromium.org
Components: -Internals>Network>DataProxy
Owner: jegray@chromium.org
I think Jered was already working on this.  Issue 908985  tracks the work to reduce the computation as well as to move the computation off the main thread to background thread.

Comment 5 by tbansal@chromium.org, Nov 29

Status: Assigned (was: Untriaged)

Comment 6 by ssid@chromium.org, Nov 29 

some stats based on Dev releases: 
3608 shows very less impact of the jank affecting 0.5% users.
3610 shows big impact affecting 8-9% users.
latest 3623 shows it affects 35% of users (not enough data yet to be certain).

Is there is a Finch experiment that enables the service, so we can make sure there is no regression because of this feature?

Can we make sure M72 is not branched with this regression?

Comment 7 by tbansal@chromium.org, Nov 29 

I do not know of any difference between 3608 and 3610. There is also a finch experiment related to a part of this feature which would cause Canary/Dev users to see more jank than Beta/Stable users.
https://uma.googleplex.com/p/chrome/variations/?sid=b446513c2bf240c9c9c404a0a66dd4c9

> Can we make sure M72 is not branched with this regression?
Yes. The CL got reverted due to flaky browser test, but we should have a fix for it.

Comment 8 by ssid@chromium.org, Nov 29 

Thanks for your reply.

The issue affects startup janks. So, the startup metrics would be affected more than general jank metrics. It would also show up only on high end devices since there are a lot of other issues with low end ones that this won't be visible. The regression is not significant, but visible.
https://uma.googleplex.com/p/chrome/variations/?sid=8a347f06feaaae10549020c2c5c7b340

Comment 9 by tbansal@chromium.org, Nov 29 

Thanks. I think we should add those histograms to finch config so the chirp pipeline alerts in case we severely regress those.
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 29 

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

commit e72337f49c13f7fd4e874ae0c1a38fb74ba69675
Author: Jered Gray <jegray@google.com>
Date: Thu Nov 29 22:26:57 2018

Reland "Eliminate unnecessary proto::Configuration copies"

This is a reland of 1b420fdb7ff25f00b37319031d243424b1db2b0e

The flakiness in the original CL with ResourceLoadingHintsBrowserTest
and PreviewsNoScriptBrowserTest has been fixed.
RetryForHistogramUntilCountReached() now flushes the task scheduler
and keeps trying until the test times out.

Original change's description:
> Eliminate unnecessary proto::Configuration copies
>
> The previous logic in OptimizationGuideService created an
> optimization_guide::proto::Configuration object on a background thread
> and then used task posting to send it on to
> PreviewsHints::CreateFromConfig(), where it was used to create a
> PreviewsHints object on another background thread. In all, there were
> two tasks posted that included the config object as a parameter, one on
> the background thread and one on the UI thread. Including it as a
> parameter in a task requires a full copy of the protobuffer. This means
> that a full copy of the config protobuf, which can be as large as 1.8MB
> (the size of the current Brazil one on Canary), was occurring on the UI
> thread.
>
> In local performance measurements, making a single copy of a 600KB
> version of the protobuf took several milliseconds (it amounted to
> roughly 60% of the time taken by the initial component string parsing
> and 60% of the time taken by PreviewHints::CreateFromConfig()). Given
> that we were incurring the cost of two copies, one of which was on an
> extremely high priority thread, it makes sense to change the logic to
> eliminate the need for the copies.
>
> The logic has been altered so that OptimizationGuideService no longer
> immediately processes the component, but instead notifies the
> observers that it is available and allows them to trigger the
> processing. This eliminates both copies of the configuration protobuf,
> as it is now created where it is used.
>
> Additionally, OptimizationGuideServiceObservers are now immediately
> notified of the hints component when they register if one is already
> available. This will enable the PreviewsOptimizationGuide to wait until
> the HintCacheLevelDBStore is fully initialized before registering for
> the component, and in the future will potentially allow it to avoid
> processing the configuration altogether when the store already has the
> latest version cached.
>
> New unittests have been added and older ones have been updated to
> accommodate the new logic.
>
> The related browser tests have also been modified to be more robust,
> where they now wait for a signal from a local histogram indicating
> that hint processing is complete.

Bug:  908985 ,  910251 
Change-Id: I90407db4c19dac29e10f756a6de87294a9ab683b
Reviewed-on: https://chromium-review.googlesource.com/c/1355256
Commit-Queue: Jered Gray <jegray@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612408}
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/chrome/browser/component_updater/optimization_hints_component_installer.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/chrome/browser/component_updater/optimization_hints_component_installer_unittest.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/chrome/browser/previews/previews_browsertest.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/chrome/browser/previews/resource_loading_hints/resource_loading_hints_browsertest.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/BUILD.gn
[add] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/hints_component_info.h
[add] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/hints_component_util.cc
[add] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/hints_component_util.h
[add] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/hints_component_util_unittest.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/optimization_guide_service.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/optimization_guide_service.h
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/optimization_guide_service_observer.h
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/optimization_guide_service_unittest.cc
[rename] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/test_hints_component_creator.cc
[rename] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/optimization_guide/test_hints_component_creator.h
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/content/previews_hints.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/content/previews_hints.h
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/content/previews_hints_unittest.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/content/previews_optimization_guide.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/content/previews_optimization_guide.h
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/content/previews_optimization_guide_unittest.cc
[modify] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/core/BUILD.gn
[add] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/core/previews_constants.cc
[add] https://crrev.com/e72337f49c13f7fd4e874ae0c1a38fb74ba69675/components/previews/core/previews_constants.h

Comment 11 by jegray@chromium.org, Dec 3

Labels: M-72
Project Member

Comment 12 by bugdroid1@chromium.org, Dec 3 

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

commit 35dcd9d12dee7d30b40ffaabff82a653bea22420
Author: Jered Gray <jegray@google.com>
Date: Mon Dec 03 19:40:32 2018

Stop initializing HintCache on UI thread

Previously, the HintCache's initialization, which involved adding all
hints contained within a hint vector to the cache, was being handled
on the UI thread. This initialization could take a significant amount
of time and triggered the creation of a jank startup performance bug.

The logic has been modified so that the HintCache is no longer
initialized on the UI thread. It is instead initialized within its
constructor during PreviewsHints::CreateFromHintsComponent() on a
background thread.

This is accomplished via a HintCache::Data object. Once the hints are
fully added to Data, it is moved into the HintCache's constructor. The
HintCache is immutable after construction.

Not only does this remove all of the costly UI thread processing, but
it also eliminates the expensive intermediate hint vector.

Bug:  910251 
Change-Id: I04a28248ad1ce790f999da06f48e3c5aabfcb812
Reviewed-on: https://chromium-review.googlesource.com/c/1356226
Commit-Queue: Jered Gray <jegray@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613196}
[modify] https://crrev.com/35dcd9d12dee7d30b40ffaabff82a653bea22420/components/previews/content/hint_cache.cc
[modify] https://crrev.com/35dcd9d12dee7d30b40ffaabff82a653bea22420/components/previews/content/hint_cache.h
[modify] https://crrev.com/35dcd9d12dee7d30b40ffaabff82a653bea22420/components/previews/content/hint_cache_unittest.cc
[modify] https://crrev.com/35dcd9d12dee7d30b40ffaabff82a653bea22420/components/previews/content/previews_hints.cc
[modify] https://crrev.com/35dcd9d12dee7d30b40ffaabff82a653bea22420/components/previews/content/previews_hints.h
[modify] https://crrev.com/35dcd9d12dee7d30b40ffaabff82a653bea22420/components/previews/content/previews_hints_unittest.cc
[modify] https://crrev.com/35dcd9d12dee7d30b40ffaabff82a653bea22420/components/previews/content/previews_optimization_guide.cc

Comment 13 by jegray@chromium.org, Dec 6

Labels: Merge-Request-72
This bug fix needs to be merged into M-72 because without it there will be additional startup jank affecting a large percentage of users.

This fix is safe because the logic is guarded behind a Finch trial and can be remotely disabled.
Project Member

Comment 14 by sheriffbot@chromium.org, Dec 7

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 15 by gov...@chromium.org, Dec 7 

Pls merge your change to M72 branch 3626 ASAP so we can pick it up for next  Dev & Beta release. Thank you.
Project Member

Comment 16 by bugdroid1@chromium.org, Dec 7

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebc5cbaa41789268031e98e724c0046349db76b7

commit ebc5cbaa41789268031e98e724c0046349db76b7
Author: Jered Gray <jegray@google.com>
Date: Fri Dec 07 20:58:06 2018

Stop initializing HintCache on UI thread

Previously, the HintCache's initialization, which involved adding all
hints contained within a hint vector to the cache, was being handled
on the UI thread. This initialization could take a significant amount
of time and triggered the creation of a jank startup performance bug.

The logic has been modified so that the HintCache is no longer
initialized on the UI thread. It is instead initialized within its
constructor during PreviewsHints::CreateFromHintsComponent() on a
background thread.

This is accomplished via a HintCache::Data object. Once the hints are
fully added to Data, it is moved into the HintCache's constructor. The
HintCache is immutable after construction.

Not only does this remove all of the costly UI thread processing, but
it also eliminates the expensive intermediate hint vector.

Bug:  910251 
Change-Id: I04a28248ad1ce790f999da06f48e3c5aabfcb812
Reviewed-on: https://chromium-review.googlesource.com/c/1356226
Commit-Queue: Jered Gray <jegray@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613196}(cherry picked from commit 35dcd9d12dee7d30b40ffaabff82a653bea22420)
Reviewed-on: https://chromium-review.googlesource.com/c/1366792
Cr-Commit-Position: refs/branch-heads/3626@{#151}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/ebc5cbaa41789268031e98e724c0046349db76b7/components/previews/content/hint_cache.cc
[modify] https://crrev.com/ebc5cbaa41789268031e98e724c0046349db76b7/components/previews/content/hint_cache.h
[modify] https://crrev.com/ebc5cbaa41789268031e98e724c0046349db76b7/components/previews/content/hint_cache_unittest.cc
[modify] https://crrev.com/ebc5cbaa41789268031e98e724c0046349db76b7/components/previews/content/previews_hints.cc
[modify] https://crrev.com/ebc5cbaa41789268031e98e724c0046349db76b7/components/previews/content/previews_hints.h
[modify] https://crrev.com/ebc5cbaa41789268031e98e724c0046349db76b7/components/previews/content/previews_hints_unittest.cc
[modify] https://crrev.com/ebc5cbaa41789268031e98e724c0046349db76b7/components/previews/content/previews_optimization_guide.cc
Project Member

Comment 17 by cr-audit...@appspot.gserviceaccount.com, Dec 19

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ebc5cbaa41789268031e98e724c0046349db76b7

Commit: ebc5cbaa41789268031e98e724c0046349db76b7
Author: jegray@google.com
Commiter: tbansal@chromium.org
Date: 2018-12-07 20:58:06 +0000 UTC

Stop initializing HintCache on UI thread

Previously, the HintCache's initialization, which involved adding all
hints contained within a hint vector to the cache, was being handled
on the UI thread. This initialization could take a significant amount
of time and triggered the creation of a jank startup performance bug.

The logic has been modified so that the HintCache is no longer
initialized on the UI thread. It is instead initialized within its
constructor during PreviewsHints::CreateFromHintsComponent() on a
background thread.

This is accomplished via a HintCache::Data object. Once the hints are
fully added to Data, it is moved into the HintCache's constructor. The
HintCache is immutable after construction.

Not only does this remove all of the costly UI thread processing, but
it also eliminates the expensive intermediate hint vector.

Bug:  910251 
Change-Id: I04a28248ad1ce790f999da06f48e3c5aabfcb812
Reviewed-on: https://chromium-review.googlesource.com/c/1356226
Commit-Queue: Jered Gray <jegray@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613196}(cherry picked from commit 35dcd9d12dee7d30b40ffaabff82a653bea22420)
Reviewed-on: https://chromium-review.googlesource.com/c/1366792
Cr-Commit-Position: refs/branch-heads/3626@{#151}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Comment 18 by dougarnett@chromium.org, Dec 27

Status: Fixed (was: Assigned)
►

Sign in to add a comment