109963 - improve XBL string usage

Status: RESOLVED FIXED

(Core :: XBL, defect, P4)

Description

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

I was working a bit to improve string usage within XBL last night.  I'm not sure
if the end result is worth checking in.  One way to tell would be to see if
things are faster or slower -- I'm not sure which they will be.  Anyway, I'll
attach the patch

Comment 1

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

Attachment: patch

Comment 2

[John Morrison]

this patch is crashing for me on win32, current trunk, in 
|nsXBLProtoImplProperty::CompileMember|, doing |nsMemory::Free(mSetterText);|

Comment 3

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

Yeah, it's not really ready yet.  And I'm not sure whether it's worth working
on.    I should've tried running the build before attaching the patch.  I have
too many builds to remember which patch is in which build...

Comment 4

[Daniel Bratell]

Yes, it's difficult to know. I did some similar work in gfx which turned out
only moving the costs from point A to to point B.

Comment 5

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

Attachment: reduce size of mFieldText by avoiding double-on-fault

Comment 6

[jag (Peter Annema)]

Comment on attachment 57888 [details] [diff] [review]
reduce size of mFieldText by avoiding double-on-fault

r=jag

Comment 7

[David Hyatt]

sr=hyatt

Comment 8

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

Attachment 57888 [details] [diff] checked in 2001-11-15 18:17 PDT.

Comment 9

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

Attachment: 5K code size reduction

Comment 10

[David Hyatt]

sr=hyatt

Comment 11

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

Oops, I forgot to make gKeyCodes |static|.

Comment 12

[jag (Peter Annema)]

Comment on attachment 64737 [details] [diff] [review]
5K code size reduction

r=jag with the static

Comment 13

[Brendan Eich [:brendan]]

Comment on attachment 64737 [details] [diff] [review]
5K code size reduction

hyatt didn't use the patch manager, so I'm first sr= officially!

The table is almost long enough that I'd want a hash (maybe generated by the
GNU non-minimal perfect hash table generator).	Whaddya say?

sr=[e-mail] in any event.

/be

Comment 14

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

Attachment 64737 [details] [diff] (with |static| and with |strlen| changed to |strlength| since
I'm afraid it could be a macro on some platforms) checked in, 2002-01-14 06:59 PST.

Comment 15

[Brendan Eich [:brendan]]

dbaron: I pondered your use of strlen as a member name, but I think it's ok
(although easily avoided).  If strlen is #defined as a macro taking parameters,
it won't be expanded if used without a ( as the next token after its name.  If
it's #defined as a macro taking no parameters, then it'll expand to some other
name (presumably, a global function name), but only a debugger-user on the
afflicted (seriously!) platform might care.

What do you say about using a build-time perfect hash table generator?

/be

Comment 16

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

I don't think it's worth it because I doubt we call this function much.

Comment 17

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

And if we did it would be easier to just sort them alphabetically in the source
file and do a binary search.

Comment 18

[Alfred Kayser]

It seems that the last patch has allready been submitted:
According to CVS Blame:
1.47 <[e-mail]> 2002-01-14 06:59
Reduce 5K of binary code size by representing data as data rather than code. b=109963 r=jag sr=hyatt,brendan

Points remaining:
* Sort the entries, so that the matching can be binary?
* replace nsCRT::strcmp with strcmp
* the mFieldText patch, but I don't believe will improve things much.

May be close this one as fixed (it did do the codesize reduction, and
make another bug to optimize the matching loop?

Comment 19

[Alfred Kayser]

See comment #18, the patch has been committed and verified working.
As for the followup: 
1. Sorting is not worth the risk
2. nsCRT removal is a separate issue.
3. mFieldText optimisation no longer needed, assuming nsString is nowadays good enough.