You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-dev@xerces.apache.org by James Berry <jb...@criticalpath.com> on 2003/04/04 21:50:25 UTC

Re: Adding RefCount support to DOMNode

Hi Khalad,

Sorry, I didn't have the name "quite" right ;)  And since the bug doesn't
have a patch attached to it yet, it's a little hard to see... Look in the
DOMWriter code for the use of DOMNodeSPtr, which is the smart node pointer
for DOMNodes.

-jdb

On 4/4/03 11:25 AM, "Khaled Noaman" <kn...@ca.ibm.com> wrote:

> Hi James,
> 
> I have looked at the DOMWriterImpl.cpp attached to the bug. I do not
> see any reference to the nodeptr classes that you mentioned in your note
> and are proposed in the solution. I would like to get a better understanding
> on how the proposed changes are to be used.
> 
> Khaled
> 
> James Berry wrote:
> 
>> I've asked the Quark folks to modify the patch to incorporate some of the
>> various feedback.
>> 
>>  - Existence of the interfaces will be conditionalized with the
>>    macro XML_DOMREFCOUNT_EXPERIMENTAL.
>> 
>> Note that nodeptr classes are provided to ease the burden of deciding
>> whether reference counting is used or not, and to eliminate any overhead.
>> The classes internally examine the macro to determine whether to use
>> addRefcount/decRefcount or simply copy the pointer. Any code that wishes to
>> support the experimental interface, but not commit to it for all cases (or
>> risk the burden thereof), can use these classes. That is how the adoption
>> inside DOMWriter is done...this makes for a clean adoption.
>> 
>> -jdb
>> 
>> On 4/4/03 1:44 AM, "Alberto Massari" <am...@progress.com> wrote:
>> 
>>> I agree with Gareth wrt to have a mechanism to include/exclude these two
>>> new methods from the build. I'll try to summarize the reasons...
>>> 
>>> - If the methods are in the public DOMNode interface (even if the official
>>> implementation is an empty function), the client code is required to invoke
>>> them properly (e.g. when storing a pointer to a DOMNode for later
>>> processing; now the client code assumes that it will become invalid only
>>> when the DOMDocument or the XercesDOMParser objects are deleted, but at
>>> this point this is no more true). That means reviewing/rewriting code that
>>> has been written against Xerces 2.0, and this is not acceptable. You could
>>> object "if your application currently doesn't call addRef/releaseRef, you
>>> don't need to add them"; but think about a company that produces a plugin
>>> (using the Xerces DLL) being hosted in an executable that uses the same
>>> version of the Xerces DLL: if they exchange DOMNode pointers, they could be
>>> expecting a wrong behavior (like: the plugin will call addRef on the
>>> pointer I give him, so I will call releaseRef as soon as the call completes
>>> -> plugins makes booooom!)
>>> - (I didn't really check the Xerces code to see if this scenario exists)
>>> Every DOMNode stored inside Xerces objects should be addRef-ed, even if the
>>> methods are stubs, and this means a little performance penalty also for the
>>> stable/official release (it's not like calling an empty inline function,
>>> it's calling a virtual table function, i.e. the function must be called
>>> every time)
>>> 
>>> So, the options I see are:
>>> 1) we don't add addRef/releaseRef; if someone wants reference counted DOM,
>>> use the old DOM_Node. If the memory management of DOM_Node makes it slow,
>>> improve it
>>> 2) we add addRef/releaseRef, but inside a #ifdef section; all the code
>>> inside Xerces that stores copies of DOMNode objects, will have the proper
>>> calls inside the same #ifdef (or maybe store a wrapper<DOMNode> object that
>>> takes care of calling addRef/releaseRef when the proper #ifdef is used)
>>> At this point we must signal that this DLL has (or has not) the two
>>> methods; the only way it to generate a different DLL name, and use a
>>> different namespace name. As a side effect, we are placing the plumbing for
>>> the feature "Selectable Component Build (Xerces-C++ Lite)"; we could create
>>> a bunch of macros that enable/disable part of the build, generating a
>>> different library name.
>>> Examples of selections:
>>> - Include/Exclude deprecated DOM (it's more than 200K of compiled code...)
>>> - Include/Exclude proprietary extensions to DOM interface
>>> - Include/Exclude schema support
>>> - Include/Exclude DTD support
>>> 
>>> Just my € 0.02
>>> 
>>> Alberto
>>> 
>>> 
>>> 
>>> I would prefer if the addRef/releaseRef methods were inside an #ifdef
>>> 
>>> At 09.51 04/04/2003 +0100, Gareth Reakes wrote:
>>>> On Thu, 3 Apr 2003, Khaled Noaman wrote:
>>>> 
>>>>> If I understand correctly, the new proposal has nothing to do with the
>>>>> underlying DOM implementation. The proposal is to add two new
>>>>> non-standard extensions to DOMNode. My concern is that we are
>>>>> mixing implementation issues with standard spec compliance. The
>>>>> DOMNode and its underlying DOM tree represent an interface for
>>>>> the DOM spec, and the details for the implementation is left to the
>>>>> users. I agree that adding those two new methods won't affect
>>>>> performance, but I think that we should not add any implementation
>>>>> specific methods to the DOM interface.
>>>>> 
>>>>> Just my 2 cents worth...
>>>> 
>>>> 
>>>> I normally am against adding any non standard methods into the DOM
>>>> interfaces. The one area where I am not immediately against this is memory
>>>> management. We already have a non standard release call to deal with the
>>>> current model. If we want flexibility for users of xerces-c then it does
>>>> not seem unreasonable to add additional methods. As these are non standard
>>>> I would have no problem with them being in ifdefs and requiring a special
>>>> build. I know this is a frequently requested feature and considering it is
>>>> low impact I don't have an objection to giving it a try.
>>>> 
>>>> as with Khaled - just my 2 pence :)
>>>> 
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
>> For additional commands, e-mail: xerces-c-dev-help@xml.apache.org
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
> For additional commands, e-mail: xerces-c-dev-help@xml.apache.org
> 
> 

--
/**********************************
 James D. Berry
 mailto:jberry@criticalpath.com
 vox:503.265.1213 fax:503.222.3020
 **********************************/



---------------------------------------------------------------------
To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
For additional commands, e-mail: xerces-c-dev-help@xml.apache.org


Re: Adding RefCount support to DOMNode

Posted by Khaled Noaman <kn...@ca.ibm.com>.
Oops... You're right. It's my mistake. I was looking at the diff of the
attached file against the CVS copy and missed it. I noticed that the
attached copy is using an old version of DOMWriterImpl, and as you
mentioned in the bug's comments having a new patch against CVS is
desirable.

Thanks,
Khaled

James Berry wrote:

> On 4/4/03 12:09 PM, "Khaled Noaman" <kn...@ca.ibm.com> wrote:
>
> > HI James,
> >
> > The bug has a copy of DOMWriterImpl.cpp attached to it. I have looked
> > at the code and there is no use whatsoever of DOMNodeSPtr.
>
> Funny... I see it at lines 774, 859, and 932.
>
> -jdb
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
> For additional commands, e-mail: xerces-c-dev-help@xml.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
For additional commands, e-mail: xerces-c-dev-help@xml.apache.org


Re: Adding RefCount support to DOMNode

Posted by James Berry <jb...@criticalpath.com>.
On 4/4/03 12:09 PM, "Khaled Noaman" <kn...@ca.ibm.com> wrote:

> HI James,
> 
> The bug has a copy of DOMWriterImpl.cpp attached to it. I have looked
> at the code and there is no use whatsoever of DOMNodeSPtr.

Funny... I see it at lines 774, 859, and 932.

-jdb


---------------------------------------------------------------------
To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
For additional commands, e-mail: xerces-c-dev-help@xml.apache.org


Re: Adding RefCount support to DOMNode

Posted by Khaled Noaman <kn...@ca.ibm.com>.
HI James,

The bug has a copy of DOMWriterImpl.cpp attached to it. I have looked
at the code and there is no use whatsoever of DOMNodeSPtr.

Khaled

James Berry wrote:

> Hi Khalad,
>
> Sorry, I didn't have the name "quite" right ;)  And since the bug doesn't
> have a patch attached to it yet, it's a little hard to see... Look in the
> DOMWriter code for the use of DOMNodeSPtr, which is the smart node pointer
> for DOMNodes.
>
> -jdb
>
> On 4/4/03 11:25 AM, "Khaled Noaman" <kn...@ca.ibm.com> wrote:
>
> > Hi James,
> >
> > I have looked at the DOMWriterImpl.cpp attached to the bug. I do not
> > see any reference to the nodeptr classes that you mentioned in your note
> > and are proposed in the solution. I would like to get a better understanding
> > on how the proposed changes are to be used.
> >
> > Khaled
> >
> > James Berry wrote:
> >
> >> I've asked the Quark folks to modify the patch to incorporate some of the
> >> various feedback.
> >>
> >>  - Existence of the interfaces will be conditionalized with the
> >>    macro XML_DOMREFCOUNT_EXPERIMENTAL.
> >>
> >> Note that nodeptr classes are provided to ease the burden of deciding
> >> whether reference counting is used or not, and to eliminate any overhead.
> >> The classes internally examine the macro to determine whether to use
> >> addRefcount/decRefcount or simply copy the pointer. Any code that wishes to
> >> support the experimental interface, but not commit to it for all cases (or
> >> risk the burden thereof), can use these classes. That is how the adoption
> >> inside DOMWriter is done...this makes for a clean adoption.
> >>
> >> -jdb
> >>
> >> On 4/4/03 1:44 AM, "Alberto Massari" <am...@progress.com> wrote:
> >>
> >>> I agree with Gareth wrt to have a mechanism to include/exclude these two
> >>> new methods from the build. I'll try to summarize the reasons...
> >>>
> >>> - If the methods are in the public DOMNode interface (even if the official
> >>> implementation is an empty function), the client code is required to invoke
> >>> them properly (e.g. when storing a pointer to a DOMNode for later
> >>> processing; now the client code assumes that it will become invalid only
> >>> when the DOMDocument or the XercesDOMParser objects are deleted, but at
> >>> this point this is no more true). That means reviewing/rewriting code that
> >>> has been written against Xerces 2.0, and this is not acceptable. You could
> >>> object "if your application currently doesn't call addRef/releaseRef, you
> >>> don't need to add them"; but think about a company that produces a plugin
> >>> (using the Xerces DLL) being hosted in an executable that uses the same
> >>> version of the Xerces DLL: if they exchange DOMNode pointers, they could be
> >>> expecting a wrong behavior (like: the plugin will call addRef on the
> >>> pointer I give him, so I will call releaseRef as soon as the call completes
> >>> -> plugins makes booooom!)
> >>> - (I didn't really check the Xerces code to see if this scenario exists)
> >>> Every DOMNode stored inside Xerces objects should be addRef-ed, even if the
> >>> methods are stubs, and this means a little performance penalty also for the
> >>> stable/official release (it's not like calling an empty inline function,
> >>> it's calling a virtual table function, i.e. the function must be called
> >>> every time)
> >>>
> >>> So, the options I see are:
> >>> 1) we don't add addRef/releaseRef; if someone wants reference counted DOM,
> >>> use the old DOM_Node. If the memory management of DOM_Node makes it slow,
> >>> improve it
> >>> 2) we add addRef/releaseRef, but inside a #ifdef section; all the code
> >>> inside Xerces that stores copies of DOMNode objects, will have the proper
> >>> calls inside the same #ifdef (or maybe store a wrapper<DOMNode> object that
> >>> takes care of calling addRef/releaseRef when the proper #ifdef is used)
> >>> At this point we must signal that this DLL has (or has not) the two
> >>> methods; the only way it to generate a different DLL name, and use a
> >>> different namespace name. As a side effect, we are placing the plumbing for
> >>> the feature "Selectable Component Build (Xerces-C++ Lite)"; we could create
> >>> a bunch of macros that enable/disable part of the build, generating a
> >>> different library name.
> >>> Examples of selections:
> >>> - Include/Exclude deprecated DOM (it's more than 200K of compiled code...)
> >>> - Include/Exclude proprietary extensions to DOM interface
> >>> - Include/Exclude schema support
> >>> - Include/Exclude DTD support
> >>>
> >>> Just my € 0.02
> >>>
> >>> Alberto
> >>>
> >>>
> >>>
> >>> I would prefer if the addRef/releaseRef methods were inside an #ifdef
> >>>
> >>> At 09.51 04/04/2003 +0100, Gareth Reakes wrote:
> >>>> On Thu, 3 Apr 2003, Khaled Noaman wrote:
> >>>>
> >>>>> If I understand correctly, the new proposal has nothing to do with the
> >>>>> underlying DOM implementation. The proposal is to add two new
> >>>>> non-standard extensions to DOMNode. My concern is that we are
> >>>>> mixing implementation issues with standard spec compliance. The
> >>>>> DOMNode and its underlying DOM tree represent an interface for
> >>>>> the DOM spec, and the details for the implementation is left to the
> >>>>> users. I agree that adding those two new methods won't affect
> >>>>> performance, but I think that we should not add any implementation
> >>>>> specific methods to the DOM interface.
> >>>>>
> >>>>> Just my 2 cents worth...
> >>>>
> >>>>
> >>>> I normally am against adding any non standard methods into the DOM
> >>>> interfaces. The one area where I am not immediately against this is memory
> >>>> management. We already have a non standard release call to deal with the
> >>>> current model. If we want flexibility for users of xerces-c then it does
> >>>> not seem unreasonable to add additional methods. As these are non standard
> >>>> I would have no problem with them being in ifdefs and requiring a special
> >>>> build. I know this is a frequently requested feature and considering it is
> >>>> low impact I don't have an objection to giving it a try.
> >>>>
> >>>> as with Khaled - just my 2 pence :)
> >>>>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
> >> For additional commands, e-mail: xerces-c-dev-help@xml.apache.org
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
> > For additional commands, e-mail: xerces-c-dev-help@xml.apache.org
> >
> >
>
> --
> /**********************************
>  James D. Berry
>  mailto:jberry@criticalpath.com
>  vox:503.265.1213 fax:503.222.3020
>  **********************************/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
> For additional commands, e-mail: xerces-c-dev-help@xml.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: xerces-c-dev-unsubscribe@xml.apache.org
For additional commands, e-mail: xerces-c-dev-help@xml.apache.org