1480499 - Add a profiler label to gfxUserFontEntry::LoadPlatformFont with the OTHER category

Status: RESOLVED FIXED

(Core :: Graphics: Text, enhancement)

Description

[Markus Stange [:mstange]]

This stack currently shows up with the NETWORK category in the profiler:

> (anonymous namespace)::ExecuteType2CharString(ots::Font*, unsigned long, ots::CFFIndex const&, ots::CFFIndex const&, ots::Buffer*, ots::Buffer*, std::__1::stack<int, std::__1::deque<int, std::__1::allocator<int> > >*, bool*, bool*, unsigned long*)
> (anonymous namespace)::ExecuteType2CharString(ots::Font*, unsigned long, ots::CFFIndex const&, ots::CFFIndex const&, ots::Buffer*, ots::Buffer*, std::__1::stack<int, std::__1::deque<int, std::__1::allocator<int> > >*, bool*, bool*, unsigned long*)
> (anonymous namespace)::ExecuteType2CharString(ots::Font*, unsigned long, ots::CFFIndex const&, ots::CFFIndex const&, ots::Buffer*, ots::Buffer*, std::__1::stack<int, std::__1::deque<int, std::__1::allocator<int> > >*, bool*, bool*, unsigned long*)
> (anonymous namespace)::ExecuteType2CharString(ots::Font*, unsigned long, ots::CFFIndex const&, ots::CFFIndex const&, ots::Buffer*, ots::Buffer*, std::__1::stack<int, std::__1::deque<int, std::__1::allocator<int> > >*, bool*, bool*, unsigned long*)
> ots::ValidateType2CharStringIndex(ots::Font*, ots::CFFIndex const&, ots::CFFIndex const&, std::__1::map<unsigned short, unsigned char, std::__1::less<unsigned short>, std::__1::allocator<std::__1::pair<unsigned short const, unsigned char> > > const&, std::__1::vector<ots::CFFIndex*, std::__1::allocator<ots::CFFIndex*> > const&, ots::CFFIndex const*, ots::Buffer*)
> ots::OpenTypeCFF::Parse(unsigned char const*, unsigned long)
> ots::Font::ParseTable(ots::TableEntry const&, unsigned char const*, ots::Arena&)
> (anonymous namespace)::ProcessGeneric(ots::FontFile*, ots::Font*, unsigned int, ots::OTSStream*, unsigned char const*, unsigned long, std::__1::vector<ots::TableEntry, std::__1::allocator<ots::TableEntry> > const&, ots::Buffer&)
> (anonymous namespace)::ProcessTTF(ots::FontFile*, ots::Font*, ots::OTSStream*, unsigned char const*, unsigned long, unsigned int)
> ots::OTSContext::Process(ots::OTSStream*, unsigned char const*, unsigned long, unsigned int)
> gfxUserFontEntry::SanitizeOpenTypeData(unsigned char const*, unsigned int, unsigned int&, gfxUserFontType)
> gfxUserFontEntry::LoadPlatformFont(unsigned char const*, unsigned int&)
> gfxUserFontEntry::FontDataDownloadComplete(unsigned char const*, unsigned int, nsresult)
> nsFontFaceLoader::OnStreamComplete(nsIStreamLoader*, nsISupports*, nsresult, unsigned int, unsigned char const*)
> mozilla::net::nsStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult)
> nsStreamLoader::OnStopRequest
> nsCORSListenerProxy::OnStopRequest(nsIRequest*, nsISupports*, nsresult)
> mozilla::net::HttpChannelChild::DoOnStopRequest(nsIRequest*, nsresult, nsISupports*)
> mozilla::net::HttpChannelChild::OnStopRequest(nsresult const&, mozilla::net::ResourceTimingStruct const&, mozilla::net::nsHttpHeaderArray const&)
> mozilla::net::ChannelEventQueue::FlushQueue()
> mozilla::net::ChannelEventQueue::ResumeInternal()::CompleteResumeRunnable::Run()
> mozilla::SchedulerGroup::Runnable::Run()
> mozilla::tasktracer::TracedRunnable::Run()
> nsThread::ProcessNextEvent(bool, bool*)
> NS_ProcessPendingEvents(nsIThread*, unsigned int)
> nsBaseAppShell::NativeEventCallback()
> nsAppShell::ProcessGeckoEvents(void*)
> nsAppShell::ProcessGeckoEvents
> ___CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
> ___CFRunLoopDoSources0
> ___CFRunLoopRun
> _CFRunLoopRunSpecific
> RunCurrentEventLoopInMode
> ReceiveNextEventCommon
> _BlockUntilNextEventMatchingListInModeWithFilter
> __DPSNextEvent
> -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:]
> -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
> -[GeckoNSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
> -[NSApplication run]
> nsAppShell::Run()
> XRE_RunAppShell()
> MessageLoop::Run()
> XRE_InitChildProcess(int, char**, XREChildData const*)
> XRE_InitChildProcess
> 0x109601f39
> _start
> (root)

That's because nsStreamLoader::OnStopRequest has a profiler label with the NETWORK category and there's no profiler label that sets a different category on the way into font sanitation. I think gfxUserFontEntry::LoadPlatformFont would be a good place for such a label. I'm not completely sure which category would be adequate here. I think both LAYOUT and GRAPHICS would be justifiable but neither really fits, so I suggest using OTHER for now.

Comment 1

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1480499 - Add profiler label frame to gfxUserFontEntry::LoadPlatformFont with the OTHER category

Review commit: https://reviewboard.mozilla.org/r/261192/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/261192/

Comment 2

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

I also think that OTHER category is a better fit for font load.
The new label frame can be seen here: https://perfht.ml/2vwd0TZ

Comment 3

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Also unrelated but, I didn't understand why some files in gfx/thebes use 4 space indentation and some use 2 :(

Comment 4

[Markus Stange [:mstange]]

Comment on attachment 8997416 [details]
Bug 1480499 - Add profiler label frame to gfxUserFontEntry::LoadPlatformFont with the OTHER category

https://reviewboard.mozilla.org/r/261192/#review268788

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f29562384b58
Add profiler label frame to gfxUserFontEntry::LoadPlatformFont with the OTHER category r=mstange

Comment 6

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/f29562384b58