Potential jank caused by BrowserPolicyConnector |
|||||
Issue descriptionWe 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 at startup. The issue can be found in reports: 84d831b7f1f69cfa 7557f2da18e748b6 4235fd6bf348196a 6e608ee54638435b d928a79ace8f7a2f 3c31f13114f9f68b 9c11a38ac9cfc67c 59a0097389e81c81 0b0320f2bd8ab4df Go to crash/ReportID to view the traces. The stack trace that was hit the most: Unknown icu_62::UnicodeSet::add(int const*, int, signed char) icu_62::UnicodeSet::add(int, int) icu_62::UnicodeSet::applyFilter(signed char (*)(int, void*), void*, int, UErrorCode&) icu_62::UnicodeSet::applyIntPropertyValue(UProperty, int, UErrorCode&) icu_62::UnicodeSet::applyPropertyAlias(icu_62::UnicodeString const&, icu_62::UnicodeString const&, UErrorCode&) icu_62::UnicodeSet::applyPropertyPattern(icu_62::UnicodeString const&, icu_62::ParsePosition&, UErrorCode&) icu_62::UnicodeSet::applyPropertyPattern(icu_62::RuleCharacterIterator&, icu_62::UnicodeString&, UErrorCode&) icu_62::UnicodeSet::applyPattern(icu_62::RuleCharacterIterator&, icu_62::SymbolTable const*, icu_62::UnicodeString&, unsigned int, icu_62::UnicodeSet& (icu_62::UnicodeSet::*)(int), int, UErrorCode&) icu_62::UnicodeSet::applyPatternIgnoreSpace(icu_62::UnicodeString const&, icu_62::ParsePosition&, icu_62::SymbolTable const*, UErrorCode&) icu_62::UnicodeSet::applyPattern(icu_62::UnicodeString const&, UErrorCode&) icu_62::UnicodeSet::UnicodeSet(icu_62::UnicodeString const&, UErrorCode&) icu_62::RegexStaticSets::RegexStaticSets(UErrorCode*) icu_62::initStaticSets(UErrorCode&) icu_62::umtx_initOnce(icu_62::UInitOnce&, void (*)(UErrorCode&), UErrorCode&) icu_62::RegexCompile::RegexCompile(icu_62::RegexPattern*, UErrorCode&) icu_62::RegexPattern::compile(icu_62::UnicodeString const&, unsigned int, UParseError&, UErrorCode&) icu_62::RegexMatcher::RegexMatcher(icu_62::UnicodeString const&, unsigned int, UErrorCode&) policy::BrowserPolicyConnector::IsNonEnterpriseUser(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) policy::UserPolicySigninServiceBase::ShouldLoadPolicyForUser(std::__ndk1::basic_string<char, std::__ndk1::char_traits<char>, std::__ndk1::allocator<char> > const&) policy::UserPolicySigninServiceBase::InitializeForSignedInUser(AccountId const&, scoped_refptr<network::SharedURLLoaderFactory>) policy::UserPolicySigninServiceBase::InitializeOnProfileReady(Profile*) content::NotificationServiceImpl::Notify(int, content::NotificationSource const&, content::NotificationDetails const&) ProfileManager::DoFinalInit(Profile*, bool) ProfileManager::AddProfile(Profile*) ProfileManager::CreateAndInitializeProfile(base::FilePath const&) ProfileManager::GetProfile(base::FilePath const&) ProfileManager::CreateInitialProfile() ChromeBrowserMainParts::PreMainMessageLoopRunImpl() ChromeBrowserMainParts::PreMainMessageLoopRun() content::BrowserMainLoop::PreMainMessageLoopRun() int base::internal::Invoker<base::internal::BindState<int (content::BrowserMainLoop::*)(), base::internal::UnretainedWrapper<content::BrowserMainLoop> >, int ()>::RunImpl<int (content::BrowserMainLoop::* const&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<content::BrowserMainLoop> > const&, 0u>(int (content::BrowserMainLoop::* const&&&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<content::BrowserMainLoop> > const&&&, std::__ndk1::integer_sequence<unsigned int, 0u>) content::StartupTaskRunner::WrappedTask() void base::internal::Invoker<base::internal::BindState<void (net::DnsConfigService::*)(), base::internal::UnretainedWrapper<net::DnsConfigService> >, void ()>::RunImpl<void (net::DnsConfigService::*)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<net::DnsConfigService> >, 0u>(void (net::DnsConfigService::*&&)(), std::__ndk1::tuple<base::internal::UnretainedWrapper<net::DnsConfigService> >&&, std::__ndk1::integer_sequence<unsigned int, 0u>) base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base::MessageLoop::RunTask(base::PendingTask*) base::MessageLoop::DoWork() base::MessagePumpForUI::OnNonDelayedLooperCallback() base::(anonymous namespace)::NonDelayedLooperCallback(int, int, void*)
,
Oct 8
Unless something really bad happens in this pattern matching code, I would presume the problem here is in the slowness of the regexps? For the very least, we may try merging our ~20 regexps into a single one, so that we avoid multiple regexp compilations and matchings. This approach isn't compatible with the "DomainWhitelistRegexFailure" UMA that we report, but maybe we may sacrifice this UMA, since it's reporting about internal issues in the code? I'm also wondering whether we may throw away regexps at all here and do some simple ad-hoc string comparison...
,
Oct 10
It looks like BrowserPolicyConnector::IsNonEnterpriseUser() is doing regex matches for about 15 patterns in kNonManagedDomainPatterns with a given username. Can you help me understand why this might consume time? Is there some disk access involved while calling icu match?
,
Oct 10
So, do we know how much time is involved in these janks? I don't know why they would particularly take much time, but in general the ICU code is a black box to me. Also, the vast majority of those regexes are just straight string matches, so we could probably optimize them all away into string compares. Guido, maybe a reasonable noogler task?
,
Oct 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a549c5a8c0b75af885de174fe7147b25108cf2a9 commit a549c5a8c0b75af885de174fe7147b25108cf2a9 Author: Siddhartha S <ssid@chromium.org> Date: Mon Oct 15 20:01:01 2018 Add trace event in BrowserPolicyConnector Helps to measure why BrowserPolicyConnector::IsNonEnterpriseUser() sometimes takes too long to run on main thread. BUG=891476 Change-Id: I5dd76dfb22a3334dc1015cf6d6c1f23c7e6f2da0 Reviewed-on: https://chromium-review.googlesource.com/c/1280550 Reviewed-by: Maksim Ivanov <emaxx@chromium.org> Commit-Queue: Siddhartha S <ssid@chromium.org> Cr-Commit-Position: refs/heads/master@{#599726} [modify] https://crrev.com/a549c5a8c0b75af885de174fe7147b25108cf2a9/components/policy/core/browser/browser_policy_connector.cc
,
Oct 17
,
Nov 20
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ssid@chromium.org
, Oct 2