You are viewing a plain text version of this content. The canonical link for it is here.
Posted to xmlrpc-dev@ws.apache.org by ni...@uk.bnpparibas.com on 2005/09/27 13:09:44 UTC
[PATCH] suggested patches for memory issues
Hi there at the XML RPC project.
We have been using XML-RPC heavily in our current project. Thanks for your
hard work on this.
We have made a couple of modifications which we would like to submit for
your consideration - which correct a couple of memory issues we were
having:
Fix to allow last query results to be garbage collected.
-------------------------------------------------------------------------------
The class XmlRpcClientResponseProcessor currently fails to set the field
'result' to null at the end of the decodeResponse method.
The effect of this is that the results of the last query are not garbage
collected (until another query takes place) - and this is quite visible in
our client application.
We have fixed this by adding the finally block in the method - as below:
XmlRpcClientResponseProcessor class Line 59 - added finally block to set
result field to null
/**
* Decode an XML-RPC response from the specified InputStream.
*
* @param is The stream from which to read the response.
* @return The response, which will be a XmlRpcException if an
* error occured.
* @exception XmlRpcClientException
*/
public Object decodeResponse(InputStream is)
throws XmlRpcClientException
{
result = null;
fault = false;
try
{
parse(is);
if (fault)
{
return decodeException(result);
}
else
{
return result;
}
}
catch (Exception x)
{
throw new XmlRpcClientException("Error decoding XML-RPC
response", x);
}
finally
{
result = null;
}
}
Fix to allow Strings to be interned
----------------------------------------------------
We had a memory problem caused by Strings in our client application not
being interned, and made some changes so that the XML RPC processor
interned Strings (both values and the keys in Maps). This reduced our
client memory footprint by a factor of 10 from around 500 MB to 50MB in
heavy usage
To do this we changed:
DefaultTypeFactory class Line 105 - modified the createString method to
intern the String created
public Object createString(String cdata)
{
return cdata.intern();
}
XmlRpc class Line 607 in startElement method - uncomment the line to enable
setType (otherwise the DefaultTypeFactory above is not used to generate the
String instances)
currentValue.setType (STRING);
XmlRpc class Line 758 - characterData method, add intern() to enable keys
used in maps to be pooled
case STRUCT:
// this is the name to use for the next member of this struct
nextMemberName = cdata.intern();
Thanks again,
Nick Ebbutt
This message and any attachments (the "message") is
intended solely for the addressees and is confidential.
If you receive this message in error, please delete it and
immediately notify the sender. Any use not in accord with
its purpose, any dissemination or disclosure, either whole
or partial, is prohibited except formal approval. The internet
can not guarantee the integrity of this message.
BNP PARIBAS (and its subsidiaries) shall (will) not
therefore be liable for the message if modified.
**********************************************************************************************
BNP Paribas Private Bank London Branch is authorised
by CECEI & AMF and is regulated by the Financial Services
Authority for the conduct of its investment business in the
United Kingdom.
BNP Paribas Securities Services London Branch is authorised
by CECEI & AMF and is regulated by the Financial Services
Authority for the conduct of its investment business in the
United Kingdom.
BNP Paribas Fund Services UK Limited is authorised and
regulated by the Financial Services Authority.
Re: [PATCH] suggested patches for memory issues
Posted by Jochen Wiedmann <jo...@gmail.com>.
Hi, Rory,
first of all, I support Henri's suggestion to post context diffs.
Besides, I strongly advice you to split your suggestions into various
separate postings, that can be handled one by one. Even better: How
about opening a Jira issue for each separate item?
Regardless of the above, here are my impressions:
> The class XmlRpcClientResponseProcessor currently fails to set the field
> 'result' to null at the end of the decodeResponse method.
I understand, that this has no side effects and will possibly help in
rare situations. No problem to accept.
> We had a memory problem caused by Strings in our client application not
> being interned, and made some changes so that the XML RPC processor
> interned Strings (both values and the keys in Maps).
To me, the value of intern() depends *very* clearly on the application.
Other applications will definitely see a negative impact.
I'd vote against a mandatory change. A flag "useIntern" or whatever
seems very special. Do you see a chance to change your patch along the
following line:
- All required handling is done in the type factory.
- The DefaultTypeFactory works basically as it is now.
- There is a new InterningTypeFactory which does what you want
Jochen
Re: [PATCH] suggested patches for memory issues
Posted by Henri Gomez <he...@gmail.com>.
Thanks for the report Nick.
Could you use the diff -Nru so we could see easily the update ?
BTW, happy to see someone using ASF XML-RPC, working in the same
professional area than me :)
Regards
2005/9/27, nick.ebbutt@uk.bnpparibas.com <ni...@uk.bnpparibas.com>:
> Hi there at the XML RPC project.
>
> We have been using XML-RPC heavily in our current project. Thanks for your
> hard work on this.
> We have made a couple of modifications which we would like to submit for
> your consideration - which correct a couple of memory issues we were
> having:
>
>
> Fix to allow last query results to be garbage collected.
> -------------------------------------------------------------------------------
>
> The class XmlRpcClientResponseProcessor currently fails to set the field
> 'result' to null at the end of the decodeResponse method.
> The effect of this is that the results of the last query are not garbage
> collected (until another query takes place) - and this is quite visible in
> our client application.
> We have fixed this by adding the finally block in the method - as below:
>
> XmlRpcClientResponseProcessor class Line 59 - added finally block to set
> result field to null
>
> /**
> * Decode an XML-RPC response from the specified InputStream.
> *
> * @param is The stream from which to read the response.
> * @return The response, which will be a XmlRpcException if an
> * error occured.
> * @exception XmlRpcClientException
> */
> public Object decodeResponse(InputStream is)
> throws XmlRpcClientException
> {
> result = null;
> fault = false;
> try
> {
> parse(is);
> if (fault)
> {
> return decodeException(result);
> }
> else
> {
> return result;
> }
> }
> catch (Exception x)
> {
> throw new XmlRpcClientException("Error decoding XML-RPC
> response", x);
> }
> finally
> {
> result = null;
> }
> }
>
>
>
> Fix to allow Strings to be interned
> ----------------------------------------------------
>
> We had a memory problem caused by Strings in our client application not
> being interned, and made some changes so that the XML RPC processor
> interned Strings (both values and the keys in Maps). This reduced our
> client memory footprint by a factor of 10 from around 500 MB to 50MB in
> heavy usage
>
> To do this we changed:
>
>
> DefaultTypeFactory class Line 105 - modified the createString method to
> intern the String created
>
> public Object createString(String cdata)
> {
> return cdata.intern();
> }
>
>
>
> XmlRpc class Line 607 in startElement method - uncomment the line to enable
> setType (otherwise the DefaultTypeFactory above is not used to generate the
> String instances)
>
> currentValue.setType (STRING);
>
>
>
> XmlRpc class Line 758 - characterData method, add intern() to enable keys
> used in maps to be pooled
>
> case STRUCT:
> // this is the name to use for the next member of this struct
> nextMemberName = cdata.intern();
>
>
>
> Thanks again,
>
>
> Nick Ebbutt
>
>
>
> This message and any attachments (the "message") is
> intended solely for the addressees and is confidential.
> If you receive this message in error, please delete it and
> immediately notify the sender. Any use not in accord with
> its purpose, any dissemination or disclosure, either whole
> or partial, is prohibited except formal approval. The internet
> can not guarantee the integrity of this message.
> BNP PARIBAS (and its subsidiaries) shall (will) not
> therefore be liable for the message if modified.
>
> **********************************************************************************************
>
> BNP Paribas Private Bank London Branch is authorised
> by CECEI & AMF and is regulated by the Financial Services
> Authority for the conduct of its investment business in the
> United Kingdom.
>
> BNP Paribas Securities Services London Branch is authorised
> by CECEI & AMF and is regulated by the Financial Services
> Authority for the conduct of its investment business in the
> United Kingdom.
>
> BNP Paribas Fund Services UK Limited is authorised and
> regulated by the Financial Services Authority.
>