183373 - Add shared empty string accessors.

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Johnny Stenback (:jst)]

This came up a while back in a discussion between darin, jag, and myself (at
least). In the current string library there's currently no way to access a
shared empty string object. Because of this, every callsite where an empty
string object is needed one needs to be created, when a shared existing one
could be used in stead. While creating empty string objects isn't all that
expensive, it's still vasteful to sprincle that code all over when we could
isolate it into one place. Since nobody else went ahead and wrote the code for
this and I got tired of waiting, I decided to jump in. Patch coming up.

Comment 1

[Johnny Stenback (:jst)]

Attachment: const nsA[C]String& GetEmpty[C]String(), scc style, n' all :-)

From looking at objdump output of optimized code on RH8 (using gcc 3.2), it
looks like this saves 7 instructions per callsite over using nsString(), and 9
instructions over using nsAutoString().

I guess we could inline these methods too to return a static string, but is it
worth it?

Comment 2

[Darin Fisher]

Comment on attachment 108134 [details] [diff] [review]
const nsA[C]String& GetEmpty[C]String(), scc style, n' all :-)

might as well make these functions return nsAC?FlatString instead to enhance
their utility (unless someone can think of a reason why that would be bad).

also, if these functions were inline, wouldn't the compiler be able to
completely optimize away the call to GetEmptyString?

Comment 3

[Johnny Stenback (:jst)]

Attachment: Inline the above.

This does the same thing, but it inlines the methods. This has the benefit of
cheaper calls to these methods, but the downside is that the empty
nsDependent[C]String's will end up as static data duplicated in every library
that uses these functions.

Thoughts? What do we prefere?

Comment 4

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

What about avoiding static data and just subclassing nsDependentString with a
class that has a null character at the end, and having the buffer point to the
internal null?  Would that be faster?  (Is there a measurable difference in the
first place?)

Comment 5

[Johnny Stenback (:jst)]

I don't think any of this will be measurable in speed in any place where it
matters and where we use nsString() or any of it's friends today, what we could
save on is code size, and I doubt that doing that will save us anything in size
(and my own silly little test shows this too). Also, I was unable to write such
a class w/o using a static buffer to hold the null character due to member
initializers always being run *after* baseclass initializers are run (i.e. I was
unable to ensure my inline null-buffer was nulled out before the
nsDependentString's ctor was run).

Code size wise (and speed too, for that matter) I think our best option is the
inline version in attachment #108174 [details] [diff] [review].

Comment 6

[Darin Fisher]

Comment on attachment 108174 [details] [diff] [review]
Inline the above.

how about this:

inline const nsAFlatString& GetEmptyString()
{
  static const nsDependentString sEmptyString((PRUnichar *)L"", 0);
  return sEmptyString;
}

inline const nsAFlatCString& GetEmptyCString()
{
  static const nsDependentCString sEmptyCString("", 0);
  return sEmptyCString;
}

Comment 7

[Boris Zbarsky [:bzbarsky]]

That will fail to compile on Linux and some other platforms in a useful
fashion.. (since sizeof(wchar_t) != sizeof(PRUnichar)).

Comment 8

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

Can NS_LITERAL_STRING not be used here?

Comment 9

[Darin Fisher]

>That will fail to compile on Linux and some other platforms in a useful
>fashion.. (since sizeof(wchar_t) != sizeof(PRUnichar)).

why?  isn't sizeof(wchar_t) >= sizeof(PRUnichar) everywhere?  or are there 1
byte wchar_t's out there?  if so, then yeah... nevermind.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Oh, hm... I guess for an empty string that inequality really is sufficient...

Comment 11

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

Re comment 5: Can't you assign the null character in the call to the
initializer, e.g.:  

  class nsEmptyString {
    nsEmptyString : nsDependentString(&(mNullChar = char_type(0))) {}
    ...
  };

Comment 12

[Johnny Stenback (:jst)]

Attachment: Proposed EmptyString()

This patch is the latest and greatest in terms of providing a shared empty
string object that callers can use w/ very little overhead. The focus here is
to minimize the ammount of code needed where this is used, at a slight
performance cost.

Comment 13

[Johnny Stenback (:jst)]

Fixed.

Comment 14

[[e-mail]]

A quick lxr for NS_LITERAL_STRING("") shows up a few .get()s - what's the
"correct" way of writing these? Will EmptyString().get() do?

Comment 15

[Boris Zbarsky [:bzbarsky]]

Yep, that will work.