14914 - nsCharsetConverterManager::GetUnicodeEncoder/Decoder() is a performance issue.

Status: RESOLVED FIXED

(Core :: Internationalization, defect, P3)

Description

[rickg]

I've been running quantify against the layout engine lately, and I see
SetDocumentCharset() show up at the top of the function time list. Even
though it's only called once, and its F+D (function + dependency) time is
18,366.31. Please take a look to see what you can do to speed it up.

Comment 1

[Frank Tang]

I will take a look at this tomorrow in my cube. In the mean time, ricg, could
you mail me the following info you it is handy -
Calls-
Function Time
F+D Time

also same info for
nsCharsetAlias2:: nsCharsetAlias2 and
nsURLProperties:: nsURLProperties

?

I am not 100% sure why it take that long, but my guessing is the
SetDocumentCharset is depend on nsCharsetAlias2 and the constructor of
nsCharsetAlias2 is depend on nsURLProperties, and nsURLProperties is depend on a
blocking io to read the property file from the file system. Although
nsCharsetAlias2 is a singleton and cached by the service manager, it still take
a long time to load in the first time.

If this is what happen here, I will try the following thing to fix it-
1. In the Apprunner init code (way before we really need to use it) , add one
line to referred to the nsCharsetAlias2(through service manager)

2. change the nsCharsetAlias2 to file a seperate thread to load the property
file and return immediately. Of course, we need to put some thread protection
code there to make the implementatio multi-thread safe.
3. When we really need to access that information, blocked untill 2 finished.

Comment 2

[leger]

Putting on [Perf] radar.

Comment 3

[Frank Tang]

mark it as M11

Comment 4

[Frank Tang]

here is the data I collect

I focus on nsParser::Parser and nsScanner::SetDocumentCharset 's F+D time is
73.5098% of focus

Inside nsScanner::SetDocumentCharset (name, percent, calls, propagated time)
nsCharsetConverterManager::GetUnicodeDecoder	64.9289	4	314.6108
nsServiceManager::GetService	9.6829	8	46.9182
nsString::=	9.3398	4	45.2555
nsServiceManager::ReleaseService	8.8541	8	42.9022
nsCharsetAlias2::GetPreferred	4.4316	4	21.4731
nsAutoString::nsAutoString	1.2313	4	5.9661
nsCharsetAlias2::Equals	1.2152	4	5.8882
nsAutoString::~nsAutoString	0.1153	4	0.5589
nsCOMTypeInfo<nsICharsetConverterManager>::GetIID	0.0148	4	0.0719

Inside nsCharsetConverterManager::GetUnicodeDecoder. only one function-
nsCharsetConverterManager::GetCharsetConverter	99.9518	4	314.4591

Inside Function:	nsCharsetConverterManager::GetCharsetConverter
It take 47.7061% of Focus [nsParser::Parser]
and
nsCharsetConverterManager::GetCharsetName	31.1557	4	97.9721
nsString::EqualsIgnoreCase	23.5464	204	74.0439
nsString::`scalar deleting destructor'	22.3233	4	70.1976
nsComponentManager::CreateInstance	21.1896	4	66.6327

So it is clear that the bottleneck is in
nsCharsetConverterManager::GetCharsetConverter

I then take a look at the implementation of
nsCharsetConverterManager::GetCharsetConverter , the implement use linear search
through a size 100 array and construct/destruct the nsString inside the loop.

I think we should do the rewrite nsCharsetConverterManager::GetCharsetConverter

Comment 5

[Frank Tang]

Since we found 47% of the nsParser::Parser time is in nsCharsetConverterManger,
I rename the Summary from "nsScanner::SetDocumentCharset() is a performance
issue." to "nsCharsetConverterManager::GetUnciodeEncoder/Decoder is a
performance issue"
Here is my rewrite of nsCharsetConverterManager::GetUnicodeEncoder/Decoder.
I get rid of those string comparision and use progid directly. Notice that you
need to pick up the fix from 15647 before you apply this change.

I have test this on window, it functional. However, I have not measure the
performance yet. I will do that tomorrow morning.

In the mean time, cata, could you review the code ?

I will attache performance number tomorrow when I can access my performance
machine.


Z:\mozilla\intl\uconv\src>cvs diff -c nsCharsetConverter*.cpp
Index: nsCharsetConverterManager.cpp
===================================================================
RCS file: /m/pub/mozilla/intl/uconv/src/nsCharsetConverterManager.cpp,v
retrieving revision 1.32
diff -c -r1.32 nsCharsetConverterManager.cpp
*** nsCharsetConverterManager.cpp       1999/09/30 21:10:59     1.32
--- nsCharsetConverterManager.cpp       1999/10/06 07:26:27
***************
*** 130,140 ****
    nsICharsetConverterInfo * GetICharsetConverterInfo(ConverterInfo * ci,
        PRInt32 aIndex, PRInt32 * aSize);

-   /**
-    * General method for finding and instantiating a Converter.
-    */
-   nsresult GetCharsetConverter(const nsString * aSrc, void ** aResult,
-       const nsCID * aCID, const ConverterInfo * aArray, PRInt32 aSize);

  public:

--- 130,135 ----
***************
*** 378,408 ****
    return NULL;
  }

- nsresult nsCharsetConverterManager::GetCharsetConverter(
-                                     const nsString * aSrc,
-                                     void ** aResult,
-                                     const nsCID * aCID,
-                                     const ConverterInfo * aArray,
-                                     PRInt32 aSize)
- {
-   nsresult res = NS_ERROR_UCONV_NOCONV;
-   nsString * str;
-   GetCharsetName(aSrc, &str);
-
-   *aResult = NULL;
-   for (PRInt32 i=0; i<aSize; i++) if
(str->EqualsIgnoreCase(*(aArray[i].mCharset))) {
-     res =
nsComponentManager::CreateInstance(*(aArray[i].mCID),NULL,*aCID,aResult);
-     break;
-   }
-
-   delete str;
-
-   // well, we didn't found any converter. Damn, life sucks!
-   if ((*aResult == NULL) && (NS_SUCCEEDED(res)))
-     res = NS_ERROR_UCONV_NOCONV;
-
-   return res;
- }

  //----------------------------------------------------------------------
  // Interface nsICharsetConverterManager [implementation]
--- 373,378 ----
***************
*** 411,438 ****
                                           const nsString * aDest,
                                           nsIUnicodeEncoder ** aResult)
  {
    nsresult res;
!   if (!mMappingDone) {
!     res = CreateMapping();
!     if NS_FAILED(res) return res;
!   }
!
!   return GetCharsetConverter(aDest, (void **) aResult, &kIUnicodeEncoderIID,
!       mEncArray, mEncSize);
  }

  NS_IMETHODIMP nsCharsetConverterManager::GetUnicodeDecoder(
                                           const nsString * aSrc,
                                           nsIUnicodeDecoder ** aResult)
  {
    nsresult res;
!   if (!mMappingDone) {
!     res = CreateMapping();
!     if NS_FAILED(res) return res;
!   }
!
!   return GetCharsetConverter(aSrc, (void **) aResult, &kIUnicodeDecoderIID,
!       mDecArray, mDecSize);
  }

  NS_IMETHODIMP nsCharsetConverterManager::GetEncodableCharsets(
--- 381,422 ----
                                           const nsString * aDest,
                                           nsIUnicodeEncoder ** aResult)
  {
+   *aResult= nsnull;
+   nsIComponentManager* comMgr;
    nsresult res;
!   res = NS_GetGlobalComponentManager(&comMgr);
!   if(NS_FAILED(res))
!      return res;
!   PRInt32 baselen = nsCRT::strlen(NS_UNICODEENCODER_PROGID_BASE);
!   char progid[256];
!   PL_strncpy(progid, NS_UNICODEENCODER_PROGID_BASE, 256);
!   aDest->ToCString(progid + baselen, 256 - baselen);
!   res = comMgr->CreateInstanceByProgID(progid,NULL,
!                  kIUnicodeEncoderIID ,(void**)aResult);
!   if(NS_FAILED(res))
!     res = NS_ERROR_UCONV_NOCONV;
!   return res;
  }

  NS_IMETHODIMP nsCharsetConverterManager::GetUnicodeDecoder(
                                           const nsString * aSrc,
                                           nsIUnicodeDecoder ** aResult)
  {
+   *aResult= nsnull;
+   nsIComponentManager* comMgr;
    nsresult res;
!   res = NS_GetGlobalComponentManager(&comMgr);
!   if(NS_FAILED(res))
!      return res;
!   PRInt32 baselen = nsCRT::strlen(NS_UNICODEDECODER_PROGID_BASE);
!   char progid[256];
!   PL_strncpy(progid, NS_UNICODEDECODER_PROGID_BASE, 256);
!   aSrc->ToCString(progid + baselen, 256 - baselen);
!   res = comMgr->CreateInstanceByProgID(progid,NULL,
!                  kIUnicodeDecoderIID,(void**)aResult);
!   if(NS_FAILED(res))
!     res = NS_ERROR_UCONV_NOCONV;
!   return res;
  }

  NS_IMETHODIMP nsCharsetConverterManager::GetEncodableCharsets(

Comment 6

[Frank Tang]

The patch have been check in as r.133. Mark this as fix. Please reopen it if it is still your performance problem

Comment 7

[Frank Tang]

my check in cause 17811. I back out the check in so I need to reopen this. Mark it as M12

Comment 8

[leger]

Clearing Fixed resolution due to reopen of this bug.

Comment 9

[Frank Tang]

check in the fix again after nhotta change mail/news code.

Comment 10

[Frank Tang]

reassign the qa contact to rickg.