235499 - re-enable string buffer sharing between C++ and JS in xpconnect

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Darin Fisher]

It should be straight-forward to allow one of our new string buffers to be
referenced by an "external" JS string.  We just need to provide an API that
exposes the reference counting details.

Going the other way, from JS to C++, might not be possible given that the new
string code does not support the notion of a string handle allocated separately
from the string data.  (And, I'm not sure we want to try to make it handle
that.)  However, jst and I spoke about possibly using something like
nsAdoptingString to enable one level of sharing:

  We add the concept of a dtor-method on nsAdoptingString, add a class flag so 
  nsSubstring knows to call the dtor-method to free mData, and then implement a 
  dtor-method that does the right GC magic.

More details later.  This bug is really just a placeholder at the moment.

Comment 1

[Darin Fisher]

Attachment: v1 patch

Comment 2

[Darin Fisher]

The v1 patch is fairly straightforward.  It just exposes some of the
nsStringHeader methods with a slight bit of abstraction and some documentation.

It might be good to make nsAttrValue use one of these buffers too in order to
facilitate string sharing for set/getAttribute since those are probably huge
string consumers in the DOM API.

Also, please ignore the printf line in xpcstring.cpp.  I didn't mean to leave
that in the patch.

Comment 3

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

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

xpcom/string/public/nsStringBuffer.h
+/**
+ * This file declares methods that may be used to access the underlying
+ * buffer from a nsTAString instance.
+ */

Can you format this as:
/**
 * @file
 * This file declares ...

doxygen will be happier :-)

Comment 4

[Darin Fisher]

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

i found a critical bug with this patch.

Comment 5

[Darin Fisher]

Attachment: prototype patch

Here's another prototype patch.  This one makes nsAttrValue store sharable
string buffers, so this should greatly reduce the cost of calling getAttribute
on a DOM element.  There's a bunch of clean up I'd like to do before landing
this patch.  I'm not sure that there is any real value to the indirect API that
I have written here.  It might be better to just expose the nsStringHeader
class directly.  Maybe that class should be renamed to nsStringBuffer.	That
would allow for more inlining.	It also might be worth it to store the length
of the string buffer in the buffer itself instead of relying on the owner to
remember the length.  At any rate, this patch should be suitable for
performance testing.  It'd be nice to see what kind of win this gives before
investing more time into it.

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Some random comments:

It might be good to have something similar to nsCOMPtr for nsStringBuffer so
that you don't have to manually refcount (since people will probably forget and
we'll get leaks). With the proper syntax you might even be able to use
nsRefPtr<nsStringBuffer> directly.

I would slash nsCheapStringBufferUtils even harder. The names aren't really
logical any more, and it should use nsStringBuffer* as argument rather then
PRUnichar*. In fact the entire class could maybe be replaced by nsCheapString
and using the NS_StringBuffer functions directly in nsAttrValue.
This could be left for later of course. There's work that needs to be done with
nsAttrValue anyway (bug 232706).


Also, just so there's no missunderstandings: getAttribute probably won't get
meaningfully faster. All security checks and all the code and string-handling
before we get to the actual nsAttrValue will probably dwarf the actual
nsAttrValue handling.
However GetAttr should get a whole lot faster. Thankfully this is more important
since that's what's used during page layout and in a lot of other cases.

Comment 7

[Darin Fisher]

Thanks for the comments Jonas.  Boris told me that he intends to run some
performance tests on this, so it'll be interesting to see his results.  Just
looking at the string stats that are output at the end of a run, it looks like
the number of string buffers shared increases drammatically with this patch (by
about 40%), whereas the number of string buffers allocated only increases
slightly (by about 10%).  So, it'll be interesting to see how those numbers
correlate to actual performance gains.

Comment 8

[Darin Fisher]

> the number of string buffers shared increases drammatically with this patch (by
> about 40%)

Nevermind, that stat is counting all AddRef calls.  This API introduces some
additional AddRef overhead, so it is going to skew the AddRef total.  That total
no longer correctly counts "buffers shared instead of copied" as it originally did.

Comment 9

[Darin Fisher]

better stats:

7% increase in allocations, and 15% increase in sharing

Comment 10

[Boris Zbarsky [:bzbarsky]]

I did profile this, but the data looks pretty noisy... Not very useful, I'm
afraid.  One interesting thing is the breakdown of time spent under
nsGenericHTMLElement::GetAttr with this patch:

44257  69      852 nsGenericHTMLElement::GetAttr(int, nsIAtom*, nsAString&) const
                437 .L149
                 88 .L147
                 70 nsAString::SetLength(unsigned)
                 50 NS_StringBufferSet(nsAString&, nsStringHeader*, unsigned)
                 42 NS_StringBufferStorageSize(nsStringHeader*)
                 23 nsAttrAndChildArray::GetAttr(nsIAtom*, int) const
                 18 nsAttrValue::ToString(nsAString&) const
                 18 nsAString::Assign(nsAString const&)
                 13 nsAttrValue::Type() const
                  9 __i686.get_pc_thunk.bx
                  4 _init
                  4 nsAString::~nsAString()
                  3 _init
                  2 nsSubstring::SetLength(unsigned)
                  2 __i686.get_pc_thunk.bx

 40903  25      437 .L149
                164 NS_StringBufferSet(nsAString&, nsStringHeader*, unsigned)
                105 nsAString::Assign(nsAString const&)
                 51 nsSubstring::Assign(nsAString const&)
                 43 PR_AtomicIncrement
                 23 nsAString::~nsAString()
                 20 __i686.get_pc_thunk.bx
                  5 _init
                  1 NS_StringBufferStorageSize(nsStringHeader*)

And things like:

  4684  90      214 NS_StringBufferSet(nsAString&, nsStringHeader*, unsigned)
                 51 _PR_x86_AtomicIncrement
                 27 nsAString::~nsAString()
                 16 __i686.get_pc_thunk.bx
                 13 nsSubstring::Finalize()
                  9 PR_AtomicIncrement

The time actually spent under nsAString::Assign did drop a good bit (factor of
2) in the new code, but it looks like the NS_StringBufferSet time may well be
offsetting that.

For what it's worth, my testcase was:

<script>
function runTest() {
  var div = document.getElementById("div");
  var start = Date.now();
  JProfStartProfiling();
  for (var i = 0; i < 30000; ++i) {
    var attr = div.getAttribute("test");
  }
  JProfStopProfiling();
  alert(Date.now() - start);
}
</script>
<div id="div" test="This is a test">
<input type="button" value="Click to test" onclick="runTest()">
</div>

Comment 11

[Darin Fisher]

sounds like that part of sicking's guess was right on then.  there's much more
to getAttribute then simply returning a string value.  thanks bz for running
that profile.

Comment 12

[Boris Zbarsky [:bzbarsky]]

I did some more testing and careful reading of the results.  The time spent
under GetAttr() is about 600 hits on that testcase without this patch, about 
850 hits with the patch.  The time spent under GetAttribute is about 3100 hits
without the patch (most of it converting the string attrname into an atom and
such) and 4200 hits with the patch (time in InternalGetExistingAttrNameFromQName
jumped by 50%), and the total time spent on the testcase is 25000 hits or so
(for example, the DOM security checks take about twice as much time as the
GetAttr call, and then there's the actual JS execution, XPConnect stuff, etc).

One thing I did find is that the time for XPCStringConvert::ReadableToJSString
dropped by a factor of 3 with this patch and the time under
XPCStringConvert::JSStringToReadable dropped by a factor of 2 (both changes
together corresponded to about 2-3 times the total time spent in GetAttr).

It does look from this profile that the time taken under GetAttr() increased
with this patch, though, due to more string stuff.  The time increase for
InternalGetExistingAttrNameFromQName also seems to be due to slower string stuff
(this function makes a copy of the string and lowercases it before atomizing,
for HTML).

Comment 13

[Darin Fisher]

so, perhaps it would be worth it to inline some of the string stuff as i
mentioned earlier.  in particular, we could easily inline the following methods:

  NS_StringBufferStorageSize
  NS_StringBufferToData
  NS_StringBufferFromData

i'm not sure why XPCStringConvert::JSStringToReadable would have changed.

it would be interesting to figure out why InternalGetExistingAttrNameFromQName
became more expensive.

Comment 14

[Boris Zbarsky [:bzbarsky]]

> i'm not sure why XPCStringConvert::JSStringToReadable would have changed.

Not sure, but before:

 29138  60      624 XPCStringConvert::JSStringToReadable(JSString*)
                389 operator new(unsigned)
                 74 __libc_malloc
                 45 __i686.get_pc_thunk.bx
                 41 JS_GetStringChars
                  5 __i686.get_pc_thunk.bx
                  3 _init
                  3 JS_GetStringLength
                  3 _init
                  1 js_GetStringChars

After:

 27857  21      352 XPCStringConvert::JSStringToReadable(JSString*)
                251 operator new(unsigned)
                 42 __libc_malloc
                 16 __i686.get_pc_thunk.bx
                 13 JS_GetStringChars
                  2 _init
                  2 __i686.get_pc_thunk.bx
                  2 JS_GetStringLength
                  2 _init
                  1 js_GetStringChars

Half of the slowdown for InternalGetExistingAttrNameFromQName was in
AppendUTF16toUTF8, but ToLowerCase and AtomImpl::EqualsUTF8 also took more time.

Note that some of this may simply be noise, though -- the numbers are pretty low
compared to the 25000 overall hits, and this _is_ a Monte-Carlo profiler...

Comment 15

[Darin Fisher]

perhaps the increase in time spent under InternalGetExistingAttrNameFromQName is
due to nsCheapStringBufferUtils::GetDependentString and/or ::HashCode being more
expensive due to overhead of the NS_StringBuffer methods?  (those are the only
methods that expose "reading" one of these cheap string buffers.)

Comment 16

[Darin Fisher]

Attachment: prototype patch w/ more inlining

Here's an improved version of the patch that removes the NS_StringBuffer
methods and exposes the nsStringBuffer class directly.	It allows for more
inlining.  I still need to address sicking's comments in nsHTMLValue.h and it
may be possible to leverage nsStringBuffer::Realloc in CopyToExistingBuffer.

Comment 17

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I'm confused by boris data. The only thing I can think of getting slower with
this patch is that nsAttrValue::GetStringValue now calls addref on the
stringbuffer, which might be slower then what we did before. However that
codepath should not be called at all during
InternalGetExistingAttrNameFromQName, so why that would jump 50% I don't know.
And I don't think that codepath is called during GetAttribute either, so I'm
stumped.

Everything else should be faster as far as I can see. Possibly with exception of
functioncall overhead, don't know how big effect that had.

Darin, is there any way we can avoid the extra addref in GetStringValue? Or
would that make things like
nsAutoString foo(myAttrVal->GetStringValue())
slower?

I also noted that the nsAutoString copy-ctor copies stringvalues in a suboptimal
way. See
http://lxr.mozilla.org/mozilla/source/content/base/src/nsAttrValue.cpp#149
This might be a waste of cycles with this new way of storing strings.

Comment 18

[Benjamin Smedberg]

Is the threadsafe refcnt an issue?

Comment 19

[Darin Fisher]

> Darin, is there any way we can avoid the extra addref in GetStringValue? Or
> would that make things like
> nsAutoString foo(myAttrVal->GetStringValue())
> slower?

I don't think we can avoid that call to AddRef since we need to express the fact
that we are taking ownership of the buffer.

nsAutoString foo(myAttrVal->GetStringValue()) should be improved by this change
since there should be no memcpy involved.  Instead, the nsAutoString should just
pick up a reference to the stored string buffer.


> I also noted that the nsAutoString copy-ctor copies stringvalues in a 
> suboptimal way.

SetTo(foo.GetStringValue()) should not be a problem.  it should result in
sharing of the string buffer.  the only cost should be the cost of the reference
counting and function call overhead.

Comment 20

[Darin Fisher]

> Is the threadsafe refcnt an issue?

I would expect PR_AtomicIncrement to beat out memcpy, but for very small strings
maybe that's not the case.

Comment 21

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

(In reply to comment #19)
> > Darin, is there any way we can avoid the extra addref in GetStringValue? Or
> > would that make things like
> > nsAutoString foo(myAttrVal->GetStringValue())
> > slower?
> 
> I don't think we can avoid that call to AddRef since we need to express the 
> fact that we are taking ownership of the buffer.

But we don't have to take ownership of the string. The old code used to return a
dependant-string, could the new code do that too? Or does code like
nsAutoString a(myDependantString) force a stringcopy (that's what i meant to ask
above).

> > I also noted that the nsAutoString copy-ctor copies stringvalues in a 
> > suboptimal way.
> 
> SetTo(foo.GetStringValue()) should not be a problem.  it should result in
> sharing of the string buffer.  the only cost should be the cost of the 
> reference counting and function call overhead.

Yeah, it was the refcount and functioncall overhead I was worrying about. Though
I doubt the functioncall overhead matters come to think of it. Cloning
nsAttrValues is a farily expensive operation anyway.

(In reply to comment #20)
> > Is the threadsafe refcnt an issue?
> 
> I would expect PR_AtomicIncrement to beat out memcpy, but for very small 
> strings maybe that's not the case.

If you add the alloc to that I would think PR_AtomicIncrement will always be
faster. (doesn't alloc to be threadsafe too?) DBaron did some prototyping of
non-threadsafe refcounting and it didn't seem to be a big win (bug 232927
comment 19), though it might matter more on some platforms that doesn't have
have atomic add-and-test instructions.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Frankly, given the small number of hits in the functions involved, I would say
that the difference may just be statistical fluctuation.... :(

Comment 23

[Darin Fisher]

> But we don't have to take ownership of the string. The old code used to 
> return a dependant-string, could the new code do that too? Or does code like
> nsAutoString a(myDependantString) force a stringcopy (that's what i meant to 
> ask above).

Ah, yes.. if you assign from a dependent string into another string, it will
copy the string.  So, by giving the nsStringBuffer to the nsString, and letting
it AddRef the buffer, we enable that buffer to be shared again when the owning
string is assigned into another string.

I think bz's assessment makes sense.  It's probably just a noisy measurement as
evidenced by the change in hits under JSStringToReadable.

Comment 24

[Darin Fisher]

Attachment: v3.1 patch (ready for review)

Jonas, could you please review this patch when you have time?  Thanks!

Comment 25

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I was pondering today wheather it might save a few bytes to let the buffer
refcount be 16bit rather then 32bit. I suspect that it's very rare that there
are more ten 65535 references to a single buffer, and even in the cases where
there are, we could just clone the buffer whenever needing another user of it.

Though on second thought, it's probalby not worth the complexity given the
number of places where we probably will end up calling AddRef.


 void
 nsAttrValue::ToString(nsAString& aResult) const
 {
   switch(Type()) {
     case eString:
     {
-      PRUnichar* str = NS_STATIC_CAST(PRUnichar*, GetPtr());
+      nsStringBuffer* str = NS_STATIC_CAST(nsStringBuffer*, GetPtr());
       if (str) {
         aResult = nsCheapStringBufferUtils::GetDependentString(str);

This is probably called often enough that it's worth calling
nsStringBuffer::ToString to save yourself an extra refcount. This is the most
common codepath to retrieve attribute values.

It would be neat if there was something like already_AddRefed but for strings.
Of course it's terribly complicated what with assignment into nsAStrings that
might be oldstyle strings and whatnot.


In CopyToExistingBuffer:
+    // since we are using the buffer's storage size as the length, aOldBuf
+    // would only be usable if its storage size matched the length of aStr's
+    // exactly.  that only matters of course if aStr is not sharable.
+    CopyToBuffer(aBuf, aStr);
+    Free(aOldBuf);

Is that really true? What about when the new string is smaller then the old.
Then realloc might be very fast. Of course, if realloc had to move the buffer
we'd perform an unneccesary memcpy. (I've many times wanted a function that
would resize the buffer if it could do it fast, otherwise return a newly
allocated buffer without touching the old one).


-  static PRUint32 HashCode(const PRUnichar* aBuf) {
+  static PRUint32 HashCode(nsStringBuffer* aBuf) {
     NS_ASSERTION(aBuf, "Cannot work on null buffer!");
-    return nsCRT::BufferHashCode((char*)StrPtr(aBuf),
-                                 Length(aBuf)*sizeof(PRUnichar));
+    char* data = (char*) aBuf->Data();
+    PRUint32 len = aBuf->StorageSize() - 2;
+    return nsCRT::BufferHashCode(data, len);

This isn't your doing, but would you mind making this call the version of
nsCRT::BufferHashCode that operates on PRUnichars? That should be faster on
non-8bit processors :)


It might be a good idea in the future to have two string-returning methods on
nsAttrValue. One that returns an nsCheapString, and one that returns an
nsDependantString, for the cases where you're not assinging into something else
and want to save the refcount. But it might not be worth the trouble and is
certainly something that can wait.


The comment for nsStringBuffer::Alloc contains references to the old
NS_StringBuffer* methods.

In the nsStringBuffer::ToString methods, should you make the |str| argument the
last argument since it's a return-value-as-out-argument?


I don't really understand dlldeps.cpp, but I noticed that
nsStringBuffer::ToString isn't called from in there.

In test_stringbuffer:
+    buf2 = nsStringBuffer::FromString(str);
+    if (!buf2)
+      return PR_FALSE;
+
+    PRBool rv = (buf == buf2);

No need to test bith that buf2 is non-null and that it's equal to buf. If it's
equal to buf it has to be non-null.

You should have someone else review the xpcstring.cpp changes. I don't feel I
understand that code well enough.

r=me on the rest

Comment 26

[Darin Fisher]

> I was pondering today wheather it might save a few bytes to let the buffer
> refcount be 16bit rather then 32bit. I suspect that it's very rare that there
> are more ten 65535 references to a single buffer, and even in the cases where
> there are, we could just clone the buffer whenever needing another user of it.

We're also constrained by PR_AtomicIncrement, which operates on a 32-bit integer.


> This is probably called often enough that it's worth calling
> nsStringBuffer::ToString to save yourself an extra refcount. This is the most
> common codepath to retrieve attribute values.

Good idea.  I'll make that change.


> It would be neat if there was something like already_AddRefed but for strings.
> Of course it's terribly complicated what with assignment into nsAStrings that
> might be oldstyle strings and whatnot.

Right, and people aren't supposed to need to use the nsStringBuffer.h API that
often.  So, making the API uber easy to use is probably not that important.

 
> Is that really true? What about when the new string is smaller then the old.
> Then realloc might be very fast. Of course, if realloc had to move the buffer
> we'd perform an unneccesary memcpy. (I've many times wanted a function that
> would resize the buffer if it could do it fast, otherwise return a newly
> allocated buffer without touching the old one).

You are correct.  If we called realloc, we'd be "changing" the storage size of
the string buffer.  Still, I'm not sure yet if it is worth the extra code since
the given aStr might very well be sharable.


> This isn't your doing, but would you mind making this call the version of
> nsCRT::BufferHashCode that operates on PRUnichars? That should be faster on
> non-8bit processors :)

Right, I wondered about that.  I figured there was some reason for calling the
8-bit version of the hash function.


> The comment for nsStringBuffer::Alloc contains references to the old
> NS_StringBuffer* methods.

thx.


> In the nsStringBuffer::ToString methods, should you make the |str| argument 
> the last argument since it's a return-value-as-out-argument?

yeah, that seems like a good idea.


> You should have someone else review the xpcstring.cpp changes. I don't feel I
> understand that code well enough.
> 
> r=me on the rest

ok, thanks jonas!

Comment 27

[Darin Fisher]

Attachment: xpcom part

Comment 28

[Darin Fisher]

Attachment: xpconnect part

Comment 29

[Darin Fisher]

Attachment: content part

Comment 30

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 171083 [details] [diff] [review]
xpconnect part

OK.  I think we want to use the NS_REINTERPRET_CAST everywhere, instead of just
casting C-style to PRUnichar *.  r=shaver.

Comment 31

[Darin Fisher]

> OK.  I think we want to use the NS_REINTERPRET_CAST everywhere, instead of just
> casting C-style to PRUnichar *.  r=shaver.

Yeah, good call.  I'll make that change before landing this patch.

Comment 32

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 171085 [details] [diff] [review]
content part

The nsCheapStringBufferUtils code is now moved to nsAttrValue.cpp. It's the
exact same code, just moved to a different place.

Comment 33

[Johnny Stenback (:jst)]

Comment on attachment 171085 [details] [diff] [review]
content part

In nsCheapStringBufferUtils::CopyToBuffer():

+  if (buf) {
+    if (buf->StorageSize()/2 - 1 == len) {
+      buf->AddRef();
+      aBuf = buf;
+      return;
+    }
+  }
+  buf = nsStringBuffer::Alloc((len + 1) * 2);

s/2/sizeof(PRUnichar)/ in both places.

sr=jst

Comment 34

[Darin Fisher]

fixed-on-trunk

With xpcom+xpconnect patches, there appeared to be no effect on any Tp, Txul, or
Ts.  The nsAttrValue patch is in now, so we'll see if that has any effect.

Comment 35

[Darin Fisher]

well, from what i can tell, this patch had basically no affect on our
performance metrics.  Oh well... maybe it'll be useful down the line.

Comment 36

[Darin Fisher]

It looks like this may have reduced maxheap by about 33k on Brad (from 12216894
bytes to 12182501 bytes).

Comment 37

[David G King]

This patch is causing the following build failure on Win32 gcc

e:/mozilla/source/mozilla/js/src/xpconnect/src/xpcstring.cpp: In function `void
DOMStringFinalizer(JSContext*, JSString*)':
e:/mozilla/source/mozilla/js/src/xpconnect/src/xpcstring.cpp:63: error: invalid
conversion from `jschar*' to `PRUnichar*'
e:/mozilla/source/mozilla/js/src/xpconnect/src/xpcstring.cpp: In static member f
unction `static JSString* XPCStringConvert::ReadableToJSString(JSContext*, const
 nsAString&)':
e:/mozilla/source/mozilla/js/src/xpconnect/src/xpcstring.cpp:102: error: invalid
 conversion from `PRUnichar*' to `jschar*'
make[4]: *** [xpcstring.o] Error 1
make[4]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/obj/firefox/js/sr
c/xpconnect/src'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/obj/firefox/js/sr
c/xpconnect'
make[2]: *** [libs] Error 2
make[2]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/obj/firefox'
make[1]: *** [alldep] Error 2
make[1]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/obj/firefox'
make: *** [alldep] Error 2

Comment 38

[Darin Fisher]

we discussed that problem on #developers.  dmose was looking into the problem
since it seems that the MingW build is mistakenly defining PRUnichar as wchar_t,
which for MingW is not equal to PRUint16.  that's a major problem, and it needs
to be solved in the MingW configuration.

Comment 39

[Hiro]

The following errors occur when the "ac_add_options --disable-v1-string-abi"
option is applied. 

Mac OS X 10.3.7


c++ -o nsSubstring.o -c  -DOSTYPE=\"Darwin7.7.0\" -DOSARCH=\"Darwin\"
-D_IMPL_NS_COM  -I../../../dist/include/xpcom -I../../../dist/include/string
-I../../../dist/include
-I/Users/sek/Documents/mozilla-current/camino/mozilla/dist/include/nspr       
-fPIC   -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith
-Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy
-Wno-non-virtual-dtor -Wno-long-long
-I/Developer/SDKs/MacOSX10.2.8.sdk/usr/include -O2 -fprefetch-loop-arrays
-fpascal-strings -no-cpp-precomp -fno-common -fshort-wchar
-I/Developer/SDKs/MacOSX10.2.8.sdk/Developer/Headers/FlatCarbon -pipe  -DNDEBUG
-DTRIMMED -O2 -fprefetch-loop-arrays   -DMOZILLA_CLIENT -include
../../../mozilla-config.h -Wp,-MD,.deps/nsSubstring.pp nsSubstring.cpp
In file included from nsSubstring.cpp:47:
/Developer/SDKs/MacOSX10.2.8.sdk/usr/include/stdlib.h:77: warning: redeclaration
   of C++ built-in type `wchar_t'
nsSubstring.cpp: In member function `const void* nsAStringAccessor::vtable() 
   const':
nsSubstring.cpp:136: error: `mVTable' undeclared (first use this function)
nsSubstring.cpp:136: error: (Each undeclared identifier is reported only once 
   for each function it appears in.)
nsSubstring.cpp: In static member function `static nsStringBuffer* 
   nsStringBuffer::FromString(const nsAString&)':
nsSubstring.cpp:232: error: incomplete type `nsObsoleteAString' does not have 
   member `sCanonicalVTable'
nsSubstring.cpp: In static member function `static nsStringBuffer* 
   nsStringBuffer::FromString(const nsACString&)':
nsSubstring.cpp:245: error: incomplete type `nsObsoleteACString' does not have 
   member `sCanonicalVTable'
nsSubstring.cpp: In member function `void nsStringBuffer::ToString(unsigned 
   int, nsAString&)':
nsSubstring.cpp:258: error: incomplete type `nsObsoleteAString' does not have 
   member `sCanonicalVTable'
nsSubstring.cpp: In member function `void nsStringBuffer::ToString(unsigned 
   int, nsACString&)':
nsSubstring.cpp:281: error: incomplete type `nsObsoleteACString' does not have 
   member `sCanonicalVTable'
make[5]: *** [nsSubstring.o] Error 1
make[5]: Leaving directory
`/Users/sek/Documents/mozilla-current/camino/mozilla/xpcom/string/src'

Comment 40

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

(In reply to comment #37)
> This patch is causing the following build failure on Win32 gcc

(bug 280942)