243250 - reduce size of nsStandardURL

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Darin Fisher]

nsStandardURL has a bunch of member variables that can each be represented by a
single bit or only a few bits each.  these should probably be combined into a
single PRUint32 to help reduce the size of nsStandardURL.

the member variables i'm thinking of include:

  mURLType
  mHostEncoding
  mSpecEncoding
  mMutable
  mSupportsFileURL

we could probably also pack mDefaultPort and mPort together.

these are easy things that we can do to reduce the size of nsStandardURL.  it's
also the case that there is some redundancy with the URLSegment member
variables.  many of these could be inferred from other values, so we could
probably save space by not storing all of these.  however, that's a much more
involved change.  this bug should just be able combining the fields i mentioned
above, which should save about 20 bytes per nsStandardURL.

Comment 1

[Darin Fisher]

...this bug should just be *about* combining...

Comment 2

[Darin Fisher]

actually, i wasn't thinking when i said that combining those members would save
20 bytes.  two of those members are PRPackedBool, and as for the port numbers...
-1 has significance, and so we'd need at least 17 bits to represent a port
properly.  we could do that with a PRUint16 plus a bit field, but then we'd be
checking the bit field before using the port value.

at any rate, it was easy enough to combine:

  mURLType
  mHostEncoding
  mSpecEncoding
  mMutable
  mSupportsFileURL

all into one PRUint32 by using bit-fields.  the patch is trivial.

Comment 3

[Darin Fisher]

Attachment: v1 patch

Comment 4

[Darin Fisher]

Attachment: v1.1 patch

with better comment.

Comment 5

[Darin Fisher]

Comment on attachment 148176 [details] [diff] [review]
v1.1 patch

hey boris, this is a trivial one to review.  if you really don't have time then
no problem... just let me know.  i just figured i'd ask you since you brought
up the performance impact of nsStandardURL today :-)

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 148176 [details] [diff] [review]
v1.1 patch

+    PRUint32 mHostEncoding    : 2; // eEncoding_xxx

hm... can you not use:
  nsEncodingType mHostEncoding : 2; ? that would be clearer, I think...

r=me either way

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 148176 [details] [diff] [review]
v1.1 patch

sr=bzbarsky

Comment 8

[Darin Fisher]

> hm... can you not use:
>   nsEncodingType mHostEncoding : 2; ? that would be clearer, I think...

I double check, but I think the last time I tried to do that I got an error or
warning from the compiler.

Comment 9

[Darin Fisher]

so, GCC 3.x doesn't have a problem with bitfields on an enum type.  not sure
what i was thinking....

Comment 10

[Darin Fisher]

bah, i like the idea that all of the bit fields use the same principle type name.
it makes the fields all line up, which has visual appeal, and doesn't require
someone to figure out that nsEncodingType is just an integer.  the other fields
like mURLType also take enumeration values, but we don't have an enumeration
type for it.  in short i prefer the patch as is ;-)

Comment 11

[Darin Fisher]

fixed-on-trunk

Comment 12

[benc]

for .cpp:

1.62darin%meer.net2004-05-11 11:58 fixes bug 243250 "reduce size of
nsStandardURL" r=biesi sr=bzbarsky

for .h:

1.19darin%meer.net2004-05-11 11:58 fixes bug 243250 "reduce size of
nsStandardURL" r=biesi sr=bzbarsky