Subject RE: [Firebird-Java] Possible memory leak in EncodingFactory ?
Author Rick Debay
The buffers bufferB and bufferC are not used to store intermediate
results. They are used as a work space for the encoding.
In a previous post I suggested that bufferB be removed and the new
result array be used as the working space, and bufferC be allocated off
of the stack.
Getting rid of these instance variables makes the class follow the
flyweight pattern, so caching becomes a possibility (if testing supports
it).

// original code
public byte[] encodeToCharset(String str) {
if (bufferB.length < str.length())
bufferB = new byte[str.length()];
int length = encodeToCharset(str.toCharArray(), 0, str.length(),
bufferB);
byte[] result = new byte[length];
System.arraycopy(bufferB, 0, result, 0, length);
return result;
}

// simple change
public byte[] encodeToCharset(String str)
{
byte[] result = new byte[str.length];
/*int length =*/ encodeToCharset(str.toCharArray(), 0, str.length(),
result);
return result;
}

// original code
public String decodeFromCharset(byte[] in){
if (bufferC.length < in.length)
bufferC = new char[in.length];
int length = decodeFromCharset(in, 0, in.length, bufferC);
return new String(bufferC, 0, length);
}

// simple change
public String decodeFromCharset(byte[] in)
{
byte[] bufferC = new char[in.length];
/*int length =*/ decodeFromCharset(in, 0, in.length, bufferC);
return new String(bufferC, 0, length);
}


-----Original Message-----
From: Firebird-Java@yahoogroups.com
[mailto:Firebird-Java@yahoogroups.com] On Behalf Of Roman Rokytskyy
Sent: Tuesday, June 06, 2006 2:45 AM
To: Firebird-Java@yahoogroups.com
Subject: Re: [Firebird-Java] Possible memory leak in EncodingFactory ?

(Sorry for repost - forgot the URL)

> Well, the memory problem could also be solved by removing the buffers
> (bufferB byte[128] and bufferC char[128]) from Encoding_OneByte.
>
> None of the classes in the hierarchy have a constructor, so the only
> overhead is standard object creation. So the question is if
> Concurrent.synchronizedMap's get() is more expensive than creation.
> Since there is no difference between the objects, there is no need to
> synchronize the whole function. If N threads all race to create a
> Cp1252 object, it matters not a bit which one ultimately puts in in
the Map.
> Heck, then you could use Class.forName to create the encoder, and no
> one would need to maintain that huge switch statement.
>
> BTW, I don't have CVS access, I was using Koders.com to look at the
code.

You can always use the viewcvs service from SourceForge.net:

http://firebird.cvs.sourceforge.net/firebird/client-java/

See the comment to the version 1.10 - "removed encoder caching as it
leads to stream corruption". The reason for this is the way bufferB and
bufferC are used - they are used to keep intermediate results of
computation. This prevents caching of encoders, since each object has a
state. I think this can be improved, but did not check further yet.

If someone comes with a patch that solves this problem in an efficient
manner - I will be more than grateful for the fix.

Roman