213197 - make GetUnicodeEncoder/GetUnicodeDecoder use a hashtable and cache factories

Status: RESOLVED WORKSFORME

(Core :: Internationalization, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

While bug 212475 should have fixed at least some of the performance problem,
GetUnicodeEncoder / GetUnicodeDecoder have shown up in profiles for a while.  I
have a patch to make them use a hashtable, although I didn't see an improvement
on the page load tests.  That might mean there's a problem with the patch, but
I'll attach it anyway...

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The perf test results I did, from before bug 212475 landed, were:

without:
                                                                                
    Test id: 3F109DA65B  [ done on previous day; perhaps different
                           network conditions, etc.]
    Avg. Median : 432 msec      Minimum     : 182 msec
    Average     : 441 msec      Maximum     : 1246 msec
                                                                                
    Test id: 3F11F39E42
    Avg. Median : 432 msec      Minimum     : 172 msec
    Average     : 439 msec      Maximum     : 1214 msec
                                                                                
                                                                                
with patch:
                                                                                
    Test id: 3F11F24085
    Avg. Median : 426 msec      Minimum     : 183 msec
    Average     : 433 msec      Maximum     : 1202 msec
                                                                                
    Test id: 3F11F4EBB1
    Avg. Median : 431 msec      Minimum     : 176 msec
    Average     : 436 msec      Maximum     : 1223 msec

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think I got the first serious merge conflicts with this patch from bent's work in bug 404386.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Here's the patch that applied a few days ago.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

And this is basically the same, but reverting the conflicting parts of bent's patch.  That said, some of those parts may be better than what's here.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I tried a talos try server run, but I couldn't tell if things were any faster or not.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This is updated to be on top of the patch in bug 530328.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 9

[Henri Sivonen (:hsivonen)]

No longer relevant due to encoding_rs.