Closed
Bug 6217
Opened 26 years ago
Closed 26 years ago
DOM generated code should use nsCOMPtr instead of NS_DEF_PTR
Categories
(Core :: DOM: Core & HTML, defect, P3)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: brendan, Assigned: scc-obsolete)
References
()
Details
See also http://lxr.mozilla.org/mozilla/search?string=NS_DEF_PTR
A lot of the DOM files instantiate smart pointer classes, one per level 0 class,
but never use the smart pointer type. What's more, scc@netscape.com found that
the NS_DEF_PTR macro leads to a static destructor per class, per file of bloat,
on Windows (at least, if the pointer type is used in the same file).
Some of this stuff is literally useless; the rest vidur said he'd use nsCOMPtr
instead, although if that's the case, do we need nsIPtr.h at all? Are there
cases where we want a smart-pointer (smart enough to NS_IF_RELEASE on destruct)
that's not an nsCOMPtr?
/be
Updated•26 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M7
Version: other
Updated•26 years ago
|
Summary: DOM generated code should use nsCOMPtr instead of NS_DEF_PTR
Comment 1•26 years ago
|
||
Will do this for M7.
Since Peter Linss originally came up with the NS_DEF_PTR-based smart pointer
work, it's possible that he still uses it in some of his code. If not, then I'd
agree - we could probably get rid of it.
Comment 2•26 years ago
|
||
When we aggreed to let nsCOMPtr come online Scott said that he'd convert nsIPtr
to be a wrapper for typedefing a nsCOMPtr. Apparently this didn't happen.
Updated•26 years ago
|
Assignee: vidur → scc
Status: ASSIGNED → NEW
Target Milestone: M7
Comment 3•26 years ago
|
||
I'm will momentarily check in a version of idlc that can generate code with
nsCOMPtr instead of NS_DEF_PTR.
I spent a bit of time analyzing the results and found the following with MSVC:
when compiling both debug and optimized, the size of the object files and static
libraries were a bit larger (marginally in an optimized, a bit more in debug)
for the nsCOMPtr versions. It turns out that the unused NS_DEF_PTR defined
classes are not compiled in (since they are declared in the scope of the source
file and not used, the compiler decides that code doesn't need to be generated
for them. Note that it's the compiler making the decision, not the linker - this
is true for the object files).
What I couldn't explain was the fact that the size of the DLL was identical for
both the nsCOMPtr and NS_DEF_PTR cases (this was true for both debug and
optimized). Since this didn't make sense, I had both Nisheeth and Rick Potts
look over my shoulder while I created both versions. Scott, any thoughts on why
that would be the case?
Given the results, I haven't turned on nsCOMPtr code generation in the build.
Scott, I'm reassigning this bug to you to see if you can help explain the
results. To turn on nsCOMPtr code generation in the IDL compiler, do the
following on Windows:
1) Uncomment the #define for USE_COMPTR in dom/tools/FileGen.h.
2) Make in the dom/tools directory.
3) Make in the dom/public/idl directory.
4) Make in the dom directory.
Call me to discuss this if any of this explanation doesn't make sense.
Assignee | ||
Updated•26 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•26 years ago
|
||
I'm looking at this. My first thought is that the smart pointers aren't used in
very many places. My second thought is that where they are used, only the
simplest facilities are exploited, where |nsCOMPtr| and the old macro-generated
classes generated identical code, e.g., |operator->()|. I need to look at the
right files, though, and will email Vidur to verify.
Comment 5•26 years ago
|
||
That's definitely true - smart pointers aren't used very much and are used in
very simple ways. I've used the file dom/base/src/nsJSWindow.cpp as the basis
for some of my analysis. It's one of the admittedly few files in the DOM project
that use smart pointers.
I had anticipated a large change in size because I had assumed that the unused
NS_DEF_PTR declarations were generating code. Since I've found that they aren't,
I understand that there shouldn't be a large change in code size. What's
interesting is the larger nsCOMPtr code size and the identical DLL size. Given
these, I'd like to know whether I should switch over to nsCOMPtrs or not.
Assignee | ||
Comment 6•26 years ago
|
||
We should get rid of the macro-generated smart-pointer classes because the
mozilla world is simpler if we only have one smart-pointer implementation. If
your use of smart-pointers is light enough that you are considering forgoing them
all together, that is reasonable. If you want smart-pointers, I recommend |
nsCOMPtr|. Keeping the macro classes around for the DOM will just make it too
easy for other programmers to accidently choose the old scheme over the new.
It's your call, and since it's local impact on your code is negligable, I can see
your motivation. But the global impact of using |nsCOMPtr| within the DOM makes
things that much simpler for people outside of the core Gecko team.
Assignee | ||
Comment 7•26 years ago
|
||
just waiting for Vidur's decision on how to move forward against this bug
Assignee | ||
Updated•26 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•26 years ago
|
||
Vidur tells me he has already or will immediately `throw the switch', and to mark
this bug fixed.
Brendan, can you help us verify this bug? If you agree the resolution is
correct please mark it as Verified. Thanks!
Updated•24 years ago
|
Component: DOM Level 1 → DOM HTML
Component: DOM: HTML → DOM: Core & HTML
QA Contact: stummala → general
You need to log in
before you can comment on or make changes to this bug.
Description
•