Closed Bug 188224 Opened 21 years ago Closed 10 years ago

excessive inlining in string code?


(Core :: XPCOM, defect)

Not set





(Reporter: dbaron, Assigned: dbaron)



We should look for places where we're inlining too much in string code.  I made
some comments in my review of a patch on bug 162017 that never landed.

However, what really worries me is that the one line checkin for bug 133754,
caused a 320 byte increase in code size:
----------- Output from codesize diff log ------------- 
  Overall Change in Size
  	Total:	       +320 (+324/-4)
  	Code:	       +320 (+324/-4)
  	Data:	         +0 (+0/+0)
  	Total:	       +320 (+324/-4)
  	Code:	       +320 (+324/-4)
  	Data:	         +0 (+0/+0)
  	       +320 (+324/-4)	T (CODE)
  		       +320 (+324/-4)
  			       +324	nsWindowWatcher::OpenWindowJS(nsIDO...
  			         -4	NS_NewControllerCommandManager(nsIControllerCommandManager **)
----------- End Output from codesize diff log ---------
(I was referring to bug 162017 comment 20.)
to be honest, that one line checkin should have resulted in bloat, because we're
now creating a NS_ConvertUTF8toUCS2.. 320 bytes does seem like a lot though. 
That's 320 bytes of code size, not data size.  (And anyway, any data size
measurement should be looking at the heap rather than the stack, and not
emphasizing transient objects.)
I haven't been able to reproduce this steep a code size penalty.  I tried a few
simple tests on

526844 - original
522381 - after un-inlining ctor nsDependent[C]String
521722 - after un-inlining nsAFlat[C]String::get
521594 - after un-inlining ctor nsXPIDL[C]String

The code size gain for adding new strings seemed to vary.  Optimization seems to
do weird things, too.
this actually brings me back to a warning from a long LONG time ago that John
McMullen once warned people about: he claimed that the body of any virtual
functions that was declared inline would be included with every single object
file generated

i.e. he claimed that if in nsString.h I said:

virtual const PRUnichar* get() { return mBuf; }

then the compiled body of this function would be included in the .obj for every
single .cpp file that #included nsString.h 

I had kind of scoffed at it at the time and said "how COULD that be true?" but
maybe its the only way for a compiler to guarantee that it has a pointer to a
function for that function body to put in a vtable, since it doesn't know at
compile time if that function will or wont be called? I dunno. I might be able
to discover something with codesighs (or at least msmap2tsv and related tools)
and I'll look into it, but I'm curious if anyone else has heard anything like this?
Depends on: 196506
Appears to still be valid with the new string code, given bug 345517 comment 31.
QA Contact: scc → string
I believe we re-profiled this after 2006, but in any case I don't think this is worth actively tracking.
Closed: 10 years ago
Resolution: --- → INCOMPLETE
Component: String → XPCOM
You need to log in before you can comment on or make changes to this bug.