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

Reply via email to