227240 - Provide a minimal C-style string API that hides nsAC?String details

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement, P2)

Description

[Darin Fisher]

In a nut-shell, the goal of this bug is to provide a minimal C-style API for
getting and setting the string-value of a nsAC?String reference.  Such an API
would allow embedders and component developers to divorce themselves from any
dependency on the nsAC?String ABI.

Most agree that it was a mistake to freeze nsAC?String (plus related classes),
and this work is a step toward undoing that mistake.  Of course breaking ABI
compatibility is not an option at present, but it may be a reality at some point
in the future.  This will hopefully lead to an opportunity to significantly
improve the string classes used throughout the Mozilla core without being
constrained by the current string ABI.

At the very least, I think we can provide embedders and component developers
with a much simpler string API ;-)

Comment 1

[Darin Fisher]

Attachment: v1 patch

Comment 2

[Darin Fisher]

we probably want to add stubs to the glue code to work with this API.  that can
be a follow-up patch ;-)

i've added a test program that exercises most of the API implementation.

Comment 3

[Brendan Eich [:brendan]]

This is something we'd wantin 1.4.2 and 1.6, in order to help direct people away
from the string APIs frozen in haste for mozilla1.0.  That way, the odds of us
being able to deprecate and obsolete those by 2.0 go up.

/be

Comment 4

[Doug Turner (:dougt)]

also see nsIStringService.idl

Comment 5

[Darin Fisher]

nsIStringService (being an XPCOM service) cannot be used prior to XPCOM
initialization, but XPCOM initialization requires a nsIFile parameter, which can
only be constructed from AString (or ACString) values.  therefore,
nsIStringService unfortunately isn't much of a solution :-(

Comment 6

[Doug Turner (:dougt)]

As i mentioned, there is nothing stopping you from providing a
NS_GetStringService() that returns the nsIStringService.  This would be a
special service that can be used prior to initialization.  This has been done
before -- you can use nsIFile's prior to inialization via NS_NewLocalFile().  

I believe this approach is much more consistent to what we are doing with the
XPCOM library.

However, as we also discussed, nsIStringService does heap allocation.  If that
is fixed, are there other concerns that you have wrt this approach?

Comment 7

[Mike Shaver (:shaver emeritus)]

Everyone loves XPCOM-in-C!

Comment 8

[Brendan Eich [:brendan]]

I don't love XPCOM in C, but that's not what we're doing here ;-).

It seems to me, though (and this can be checked fairly easily), that darin's
added string API code is smaller than the combination of nsStringService plus
NS_GetStringService, plus whatever else is needed to work before XPCOM is
initialized,  plus the changes to nsIStringService to support placement
allocation, remove heap allocation, and do the other things darin's code does.

C API glue almost always wins.

Who uses nsIStringService now?  Lxr shows nothing in the tree.  We should not
have both the string API code proposed here and nsIStringService; this town
ain't big enough for both of 'em.

/be

Comment 9

[Doug Turner (:dougt)]

the difference in size between what Darin has and what I proposed isn't that
big.  I prefer consistency and haven't been told why nsIStringService can't work.

Now one uses nsIStringService now.  It was just a proposal that died months ago
-- your right, which ever way we go, one needs to be removed from memory.

Comment 10

[Brendan Eich [:brendan]]

One issue that darin's proposal addresses: the fragile-ABI problems, and for
some platforms, insuperable name-mangling problems, with C++ APIs.

Another issue is the lack of opaque types in the current string APIs. We really
do not want users depending on vtables or other facts about nsA*Strings.  We do
want to make fragments a non-string special case, used only briefly when
concatenating a larger string that should be flat by the time the concatenation
is done.

/be

Comment 11

[Darin Fisher]

Doug,

I think you make a valid point, but consider these:

 (1) nsA*String are meant to be pseudo-primitive types.  Therefore, it seems
     reasonable that they might be accessed in the way I'm suggesting here --
     no extra levels of indirection.

 (2) It is common practice to expose library functions as exports from a DSO,
     so I think my proposal is pretty consistent with common practice.  I know
     XPCOM differs in this regard (w.r.t. its frozen API only).

 (3) nsIStringService incurs virtual function calls, whereas my proposal incurs
     DSO function calls.  DSO function calls are cheaper (around 30% by the 
     numbers I've seen).  This is a motivating factor in our deCOMtamification 
     effort.

Beyond your consistency argument is there any other reason to prefer
nsIStringService?  It just seems overly clumsy to go through a vtable to get at
these functions.  Why do that when you can just call the functions directly? 
What is to be gained by the additional level of indirection?  Additional layers
of indirection for "consistency" in this case doesn't make a whole lot of sense
to me.

Comment 12

[Darin Fisher]

One other point:

ELF requires vtable methods to be resolved when the DSO is loaded, whereas
normal ELF entry points are lazily loaded.  vtables are bad! ;-)

Comment 13

[Doug Turner (:dougt)]

brendan,

the abi problem isn't ours to solve.  All things are hosed when that changes.  

I do not understand your second issue.  

darin,

(1) You are correct.  That is what the nsIStringService does (kinda)
(2) I am not arguing that we shouldn't export functions from a DSO.  See comment
#6.  I just think that the XPCOM library "mostly" consist of factory exports.
(3) 30% of what?  how many calls to you expect to use this? 

How does comment #12 change when you explictly load the library with the global
flag?

Comment 14

[Darin Fisher]

> (1) You are correct.  That is what the nsIStringService does (kinda)

Not really.  It adds extra level of indirection (i.e., an extra vtable).  Why
bother with that?


> (2) I am not arguing that we shouldn't export functions from a DSO.  See 
> comment #6.  I just think that the XPCOM library "mostly" consist of factory 
> exports.

"Factory exports" ... we are doing more than just creating objects here.
What I'm proposing is akin to NSPR and most other DSOs in existance ;-)


> (3) 30% of what?  how many calls to you expect to use this? 

What I meant was: DSO calls are around 30% cheaper than virtual function calls.
 This is based on some simple measurements that bryner performed.  I don't know
the statistical accuracy of his results, and I also don't know how it would vary
under less artificial conditions (such as those of a real application).


> How does comment #12 change when you explictly load the library with the 
> global flag?

I don't know.  I lifted that information from a paper by drepper on ELF.


Doug, if you really insist on having these functions go through a vtable, then
we can (I suppose) create wrappers that expose the same API that I'm suggesting
here.  It could be part of the glue library.  But, have you thought about that
implementation in detail?  Consider what it might look like for a second.  Each
entry point would need to either call NS_GetStringService or cache a pointer to
the nsIStringService.  If the pointer is to be cached, then we need to do
something to synchronize initialization so that we handle the problem of
multiple threads racing to initialize.  Having solved that problem, we have
something that works, but now clearly we are increasing the footprint of every
single external component that wishes to use these APIs.  You can argue that
that isn't much, but it all adds up.  Moreover, there is the cost of going
through those thunks: once to call into the glue and then once to go through the
 vtable.  Compare that to a simple DSO function call!

I don't understand the emphasis on so drastically limiting the number of
exported frozen symbols.  I mean let's face it, it's not like this makes even a
dent in the total number of exported symbols from XPCOM! ;-)

I realize that it increases the size of the xpcomFunctions table that the glue
library imports (for embedders that are using the glue library), but that's ok.
 Having the functions both exported from the DSO and available via
NS_GetFrozenFunctions gives us the best of both worlds, don't you think?

There is a lot of precidence in the WIN32 world for DLLs providing both regular
exported symbols and a special symbol that returns a function table accessor for
all the exported symbols.  SECUR32.DLL is a good example of such a library.  You
can either link directly to the symbols or resolve a single entry point to get
to a function table containing all symbols.  XPCOM seems to be designed in
exactly this way.

Personally, I think there is good reason for embedding applications to bypass
the glue library and just link directly to libxpcom.  One OSX, for example, this
enables the embedded app to use "prebinding" to greatly improve performance. 
So, I like the idea of offering both a glue based (load a function table)
solution along-side a traditional DSO model (link directly to exported symbols).

Comment 15

[Brendan Eich [:brendan]]

dougt: my first point stands, ABI shifts hurt C++ more than C.  My second point
is that we don't want to expose nsString*.h and friends to embedders.  We want
opaque types.

There's no advantage to calling thru a vtable to manipulate opaque, fixed-size
types.  There's only cost, as Darin points out.  We should stop using the XPCOM
hammer for all nails, especially for strings.

/be

Comment 16

[Darin Fisher]

bryner suggested replacing ToString with variants on "operator nsA*String()" ..
i think that sounds like a great idea.

Comment 17

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

Comment on attachment 136644 [details] [diff] [review]
v1 patch

>+NS_STRINGAPI(void)
>+NS_StringContainerFree(nsStringContainer &aContainer)
>+{
>+  // call the nsString dtor
>+  nsString &s = NS_REINTERPRET_CAST(nsString &, aContainer);
>+  s.~nsString();

Reinterpret-casting to a reference makes me a little uncomfortable.  How about:

NS_REINTERPRET_CAST(nsString*, &aContainer)->~nsString();

Also, NS_StringContainerDestroy might be better than ...Free.

>+NS_STRINGAPI(void)
>+NS_StringSetData(nsAString &aStr, const PRUnichar *aBuf, PRUint32 aCount);
>+NS_STRINGAPI(void)
>+NS_StringCopy(nsAString &aDest, const nsAString &aSrc,
>+              PRUint32 aOffset, PRUint32 aCount);

default parameters for aCount, aOffset, and aCount would probably be good

Moving the ToString functions into member conversion operators sounds ok to me,
since it doesn't sound like we'll have other applicable conversion sequences
that could cause the conversion operators to be a portability problem.

Comment 18

[Brendan Eich [:brendan]]

Conversion operators?  There goes shaver's hope for an easy C XPCOM binding. ;-)

Destroy is not usually the antonym to Init.  I use Finish.  Init : Finish :: New
: Destroy (although Mozilla code mixes New and Create).  Darin, whaddya say?

/be

Comment 19

[Darin Fisher]

yeah, i prefer Finish over Destroy and Free too.  i would even go for Finalize,
but then i'm reminded of Java :-(   ... plus Finalize is long and it begs for
Init to be changed to Initialize, which is making for much to much typing.

Init and Finish :-)

if we really wanted to provide C bindings for XPCOM, we could do so, but we'd
probably want a completely separate set of headers: ones that replace
|nsAString&| with |nsAString*|, etc.

Comment 20

[Darin Fisher]

Attachment: v1.1 patch

ok, revised patch per comments from dbaron and brendan.

Comment 21

[Darin Fisher]

punt

Comment 22

[chris hofmann]

1.6-

Comment 23

[Mike Kaply [:mkaply]]

Sorry darin, we're shutting down 1.4.2 as well.

Comment 24

[Darin Fisher]

the latest patch for this bug lives on STRING_20040119_BRANCH.  so, this bug
will be resolved along with the string defragmentation work in bug 231995.

Comment 25

[Darin Fisher]

This bug was fixed with the patch that landed for bug 231995.

See xpcom/string/public/nsStringAPI.h and xpcom/string/public/nsEmbedString.h

There may still be some methods we'll want to add, but those can be added later
without affecting the current set of methods.