8702 - Performance improvement for UTF-8 converter

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P3)

Description

[Pierre Saslawsky]

Below is an excerpt from a message I posted on May 7th. I did not get any
response to this message. You can skip the beginning if you want and jump
directly to the last paragraph (the first part is about nsJSContext, not
Internationalization, but I included it so that you get an idea of the context).

It shows that when opening a new window with AppRunner, 40% of the time is spent
in nsUnicodeDecoderHelper::ConvertByTable(). Note that at that time, the command
"New Browser Window" was opening a blank window.

--------------
I profiled Viewer on a Mac while loading "test9.html" and found out that 49%
of the time is spent in nsJSContext::InitContext(nsIScriptGlobalObject*).

The table below shows that 61.8% of the time was spent inside just 8 calls to
DocumentViewerImpl::Init() and the main culprits are 4 calls to
nsJSContext::InitContext() that represent 25.1%, and 6 more calls that
represent another 24.2%.


Function                              #Calls              %Time
(...)
NET_NGLayoutConverter                     12                67.8
nsDocumentBindInfo::OnStartBinding        12                67.6
nsWebShell::Embed                          8                66.8
--DocumentViewerImpl::Init                 8                61.8
----GlobalWindowImpl::SetNewDocument       4                32.6
------nsJSContext::InitContext             4                25.1 <---
------nsJSContext::GC                      4                 6.7
----nsWebShell::GetScriptableObject        8                28.4
------nsWebShell::CreateScriptEnvironment  8                28.4
--------NS_CreateContext                   7                28.4
----------nsJSContext::InitContext         6                24.2 <---
----------nsServiceManager::GetService     6                 4.2
--DocumentViewerImpl::Release              1                 5.0


I did a similar test with "test0.html" which has only one webshell:
nsJSContext::InitContext() was called once and represented 11.9% of the CPU
time. With "test9b.html" which has 2 webshells, it was called twice for a
total of 33.1% (17.0 + 16.1).

What about AppRunner?
A profile taken when opening a second window shows that we have 2 calls
taking 17.5% (8.6 + 8.9). But... we also spend almost 42% of the CPU time
inside nsUnicodeDecoderHelper::ConvertByTable(), with especially 48000 calls
to uMapCode() and uScan(). It means that without these malicious unicode
conversions, nsJSContext::InitContext() would represent 30% (= 17.5*100/(100-
42)).
--------------

Comment 1

[Pierre Saslawsky]

Maybe you already fixed it. I just saw a checkin in nsUnicodeDecodeHelper.cpp
that refers to some caching. Besides I can no longer see a huge amount of time
spent in i18n utilities when profiling recent builds although I can't tell
whether it's because you fixed the problem or whether it's because I'm
overflowing the number of functions that can be traced by the Profiler.

Comment 2

[Pierre Saslawsky]

Changed the summary from "takes 40%" to "used to take 40%" in order to better
reflect the time spent since I did that measurement.

Comment 3

[Frank Tang]

I have not make the cache work yet. But I did improve the performace by turning
some tinyfunction into macro. (which I should do long time ago.)
Keep this bug open and mark as M10.

Comment 4

[Frank Tang]

Mark this as M12. I plan to tune the UTF-8 to unicode nsIUnicodeDecoder but I am
not sure this is still related.

Comment 5

[Frank Tang]

This seems no longer the problem. Mark it as WONTFIX. Please reopen if you can
still see the perf problem from profiler.

Comment 6

[Simon Fraser [no longer active]]

Reopening, on the basis of some performance data that I collected from a build
around 9/29 (i.e. after the macro change was supposed to have been checked in).

Profiling data was collected for first window creation. The following i18n
routines show up within the routines that take more than 1% of the time
(excluding children):

function                call count              % of time (exclusive)
uMapCode                94276                   3.5
uCheckAndScanByTable    94020                   2.5
uScan                   94397                   2.4
uMapFormate0            94244                   1.3
uScanAlways1Byte        94020                   1.3

These are all within the top 30 functions

Comment 7

[Pierre Saslawsky]

My previous comments let me think that the performance hit would be much greater

when opening subsequent windows. In the tests I made last june, the I18N

functions did not appear when opening the first window because I did not focus on

anything less than 5 or 10% of the CPU time but when opening a second window they

represented 40%. It may not be as bad nowadays but it would still be interesting

to check. Franck: you can find more info about the Profiler in

news://news.mozilla.org/37573DA1.BC07FE95%40netscape.com

Comment 8

[Frank Tang]

simon's commet make sense. keep it in my M12 list for now. I think rewrite UTF8
to Unicode conversion will solve most of the issue....

Comment 9

[Frank Tang]

We should rewrite UTF-8 converter to not call the helper function but convert by
itself. Reassign this to bobj to find someone to own this. Change it to M13

Comment 10

[bobj]

Reassigned to jbetak for M14

Comment 11

[leger]

Deferring PDT + or - until we sort key tasks for beta1.   Is there an absolute 
time vs a % time available for this?

Comment 12

[leger]

Adding [NEED INFO] to Status Summary.

Comment 13

[leger]

Per emailvcomments from jbetak:
"The last measurement indicated significant speed improvement (10- 20x) over the
previous version of the decoder. This code is called every time Mozilla opens a 
file orvacesses a URL... I counted around 100 calls when starting the browser 
and half as muchvwhen opening a new browser window. The only confusing thing was 
Pierre's claim thatvwe spend up to 40% of the time in the converter. I haven't 
been able to verify that, thevchange will result in significant performance 
improvement, it will be in one-digit % rangevfor browser-start-up and maybe 
twice that amount when opening a new browser window."


Removing [NEED INFO] 

Comment 14

[Frank Tang]

Change to summary from "" to "performance improvemenance for UTF-8 converter"

Comment 15

[leger]

Putting on PDT+ radar for beta1.

Comment 16

[[e-mail] (away - not reading bugmail)]

Attachment: UTF-8 conversion performance test file

Comment 17

[[e-mail] (away - not reading bugmail)]

I'm closing this - the changes should be in the next build. Since the profiler 
corrupted my system I was not able to quantify the 
improvement as thoroughly as I wanted to. I'm supplying a large UTF-8 test file 
though and it seems like we can achieve up to 200% 
performance improvement over the old version of the converter up to 50% 
improvement over Navigator 4.7.

Also, we need to file a new bug for the decoder buffering. If you open the test 
file with a new build or Navigator 4.x it renders 
properly. If you use an old build (e.g. 2000020310), then you will notice that 
we "eat" some parts of the HTML and the file 
rendering is disrupted. Although this buffering scheme is not used in UTF-8 
anymore, it is still needed for other decoders and should be revisited later 
(say around M20)

Comment 18

[Teruko Kobayashi]

I verified this in 2000021008 Mac build.