Closed Bug 183373 Opened 22 years ago Closed 21 years ago

Add shared empty string accessors.

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jst, Assigned: jst)

Details

Attachments

(3 files)

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.
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 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?
Attached patch Inline the above. (deleted) — Splinter Review
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?
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?)
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 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; }
That will fail to compile on Linux and some other platforms in a useful fashion.. (since sizeof(wchar_t) != sizeof(PRUnichar)).
Can NS_LITERAL_STRING not be used here?
>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.
Oh, hm... I guess for an empty string that inequality really is sufficient...
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))) {} ... };
Attached patch Proposed EmptyString() (deleted) — Splinter Review
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.
Attachment #139512 - Flags: superreview?(dbaron)
Attachment #139512 - Flags: review?(bryner)
Attachment #139512 - Flags: superreview?(dbaron) → superreview+
Attachment #139512 - Flags: review?(bryner) → review+
Fixed.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
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?
Yep, that will work.
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: