3:12 a.m.
cc'ing JBC-dev.
The stream looks good, except that getting the raw buffer would mean
that the buffer returned would "almost certainly be longer than the
data written to it" (quoted from Brian's excellent javadocs). Bela,
how would JGroups behave with this? Would it attempt to stream the
entire buffer, including uninitialized values?
Will need to see how this gets de-serialized as well. Also, since all
cacheloaders use the same marshaller, will need to see how they deal
with this as well. Any code that relies on buf.length may end up at
best writing unnecessary data and at worst breaking when deserializing.
Cheers
Manik
On 3 Jul 2008, at 20:37, Bela Ban wrote:
Good ! I see that our friend Brian added code to EBAOS to double the
byte array and - after a certain limit - grow only linearly. Good
thing, this should help keep memory consumption down even more
Manik Surtani wrote:
> Yes, good point - will investigate.
>
> On 3 Jul 2008, at 16:59, Bela Ban wrote:
>
>> At least in AbstractMarshaller:
>> public byte[] objectToByteBuffer(Object obj) throws Exception
>> {
>> ByteArrayOutputStream baos = new ByteArrayOutputStream();
>> ObjectOutputStream out =
>> ObjectSerializationFactory.createObjectOutputStream(baos);
>> objectToObjectStream(obj, out);
>> out.close();
>> return baos.toByteArray();
>> }
>>
>> Here, an ExposedByteArrayOutputStream instead of a BAOS would
>> prevent the COPY of the byte buffer. We also don't need to be
>> concerned about reentrancy as the output streams are local (stack)
>> variables...
>>
>> Maybe you guys should take a look at all occurrences of BAOS !
>>
>> --
>> Bela Ban
>> Lead JGroups / JBoss Clustering team
>> JBoss - a division of Red Hat
>>
>
> --
> Manik Surtani
> Lead, JBoss Cache
> manik(a)jboss.org
>
>
>
>
>
>
--
Bela Ban
Lead JGroups / JBoss Clustering team
JBoss - a division of Red Hat
--
Manik Surtani
Lead, JBoss Cache
manik(a)jboss.org
4:46 a.m.
New subject: Any reason not to use ExposedByteArrayOutputStream instead of ByteArrayOutputStream ?
Manik Surtani wrote:
cc'ing JBC-dev.
The stream looks good, except that getting the raw buffer would mean
that the buffer returned would "almost certainly be longer than the
data written to it" (quoted from Brian's excellent javadocs). Bela,
how would JGroups behave with this? Would it attempt to stream the
entire buffer, including uninitialized values?
Yes, good point, this is not good ! You would have to return an
org.jgroups.util.Buffer which has the byte[] buffer but also offset and
length:
public class Buffer {
private final byte[] buf;
private final int offset;
private final int length;
public Buffer(byte[] buf, int offset, int length) {
this.buf=buf;
this.offset=offset;
this.length=length;
}
public byte[] getBuf() {
return buf;
}
public int getOffset() {
return offset;
}
public int getLength() {
return length;
}
public Buffer copy() {
byte[] new_buf=buf != null? new byte[length] : null;
int new_length=new_buf != null? new_buf.length : 0;
if(new_buf != null)
System.arraycopy(buf, offset, new_buf, 0, length);
return new Buffer(new_buf, 0, new_length);
}
public String toString() {
StringBuilder sb=new StringBuilder();
sb.append(length).append(" bytes");
if(offset > 0)
sb.append(" (offset=").append(offset).append(")");
return sb.toString();
}
}
In addition, I would have to change Marshaller to use a Buffer :
public interface Marshaller {
*Buffer* objectToByteBuffer(Object obj) throws Exception;
Object objectFromByteBuffer(*Buffer* buf) throws Exception;
}
This should have far-reaching consequences as I'm not aware of anyone
else but JBossCache to use Marshaller. Or maybe, to be on the safe side,
I could create a Marshaller2 so existing code doesn't break (extending
Marshaller)... http://jira.jboss.com/jira/browse/JGRP-796That should
Will need to see how this gets de-serialized as well. Also, since
all
cacheloaders use the same marshaller, will need to see how they deal
with this as well. Any code that relies on buf.length may end up at
best writing unnecessary data and at worst breaking when deserializing.
That
shouldn't be an issue because your JBC marshalling code creates
local variables, and doesn't reuse ByteArrayOutputStream instances
--
Bela Ban
Lead JGroups / Clustering Team
JBoss - a division of Red Hat
5:28 a.m.
New subject: Any reason not to use ExposedByteArrayOutputStream instead of ByteArrayOutputStream ?
On 4 Jul 2008, at 10:46, Bela Ban wrote:
Manik Surtani wrote:
> cc'ing JBC-dev.
>
> The stream looks good, except that getting the raw buffer would
> mean that the buffer returned would "almost certainly be longer
> than the data written to it" (quoted from Brian's excellent
> javadocs). Bela, how would JGroups behave with this? Would it
> attempt to stream the entire buffer, including uninitialized values?
Yes, good point, this is not good ! You would have to return an
org.jgroups.util.Buffer which has the byte[] buffer but also offset
and length:
public class Buffer {
private final byte[] buf;
private final int offset;
private final int length;
public Buffer(byte[] buf, int offset, int length) {
this.buf=buf;
this.offset=offset;
this.length=length;
}
public byte[] getBuf() {
return buf;
}
public int getOffset() {
return offset;
}
public int getLength() {
return length;
}
public Buffer copy() {
byte[] new_buf=buf != null? new byte[length] : null;
int new_length=new_buf != null? new_buf.length : 0;
if(new_buf != null)
System.arraycopy(buf, offset, new_buf, 0, length);
return new Buffer(new_buf, 0, new_length);
}
public String toString() {
StringBuilder sb=new StringBuilder();
sb.append(length).append(" bytes");
if(offset > 0)
sb.append(" (offset=").append(offset).append(")");
return sb.toString();
}
}
In addition, I would have to change Marshaller to use a Buffer :
public interface Marshaller {
*Buffer* objectToByteBuffer(Object obj) throws Exception;
Object objectFromByteBuffer(*Buffer* buf) throws Exception;
}
But that would involve JGroups code calling Buffer.copy() again, which
is the same as using a normal BAOS. Just that it happens a bit later
in the chain of events.
This should have far-reaching consequences as I'm not aware of
anyone else but JBossCache to use Marshaller. Or maybe, to be on the
safe side, I could create a Marshaller2 so existing code doesn't
break (extending Marshaller)... http://jira.jboss.com/jira/browse/JGRP-796That
should
> Will need to see how this gets de-serialized as well. Also, since
> all cacheloaders use the same marshaller, will need to see how they
> deal with this as well. Any code that relies on buf.length may end
> up at best writing unnecessary data and at worst breaking when
> deserializing.
That shouldn't be an issue because your JBC marshalling code creates
local variables, and doesn't reuse ByteArrayOutputStream instances
I wasn't talking about reusing stream instances, I was referring to
the same problem you have above with JGroups is going to be
encountered with every cache loader impl in that the byte[] retrieved
may be larger than necessary and contain uninitialized values.
Cheers,
--
Manik Surtani
Lead, JBoss Cache
manik(a)jboss.org
6:32 a.m.
New subject: Any reason not to use ExposedByteArrayOutputStream instead of ByteArrayOutputStream ?
Manik Surtani wrote:
But that would involve JGroups code calling Buffer.copy() again,
which
is the same as using a normal BAOS. Just that it happens a bit later
in the chain of events.
No: the byte[] buffer with offset and length are passed down to the TCP
or datagram socket and *NOT* copied at all !
This should have far-reaching consequences as I'm not aware of
anyone
else but JBossCache to use Marshaller. Or maybe, to be on the safe
side, I could create a Marshaller2 so existing code doesn't break
(extending Marshaller)...
http://jira.jboss.com/jira/browse/JGRP-796That should
I wasn't talking about reusing stream instances, I was referring to
the same problem you have above with JGroups is going to be
encountered with every cache loader impl in that the byte[] retrieved
may be larger than necessary and contain uninitialized values.
So JBC has to switch to a Buffer-like implementation too. At least where
it is important. I suggest do it immediately wherever code can be
changed (e.g. internal code) and defer it to later in cases where
external code might rely on it, in order not to break anything.
We need to reduce copying wherever possible !
--
Bela Ban
Lead JGroups / Clustering Team
JBoss - a division of Red Hat
7:17 a.m.
New subject: Any reason not to use ExposedByteArrayOutputStream instead of ByteArrayOutputStream ?
On 4 Jul 2008, at 12:32, Bela Ban wrote:
Manik Surtani wrote:
> But that would involve JGroups code calling Buffer.copy() again,
> which is the same as using a normal BAOS. Just that it happens a
> bit later in the chain of events.
No: the byte[] buffer with offset and length are passed down to the
TCP or datagram socket and *NOT* copied at all !
Ok, makes more sense. Misleading copy() method on the class had me
thinking it was actually used somewhere. :-)
> This should have far-reaching consequences as I'm not aware
of
> anyone else but JBossCache to use Marshaller. Or maybe, to be on
> the safe side, I could create a Marshaller2 so existing code
> doesn't break (extending Marshaller)...
http://jira.jboss.com/jira/browse/JGRP-796That
> should
>
> I wasn't talking about reusing stream instances, I was referring to
> the same problem you have above with JGroups is going to be
> encountered with every cache loader impl in that the byte[]
> retrieved may be larger than necessary and contain uninitialized
> values.
So JBC has to switch to a Buffer-like implementation too. At least
where it is important. I suggest do it immediately wherever code can
be changed (e.g. internal code) and defer it to later in cases where
external code might rely on it, in order not to break anything.
Ok, so once we have a Marshaller interface that uses this Buffer I can
switch to it.
We need to reduce copying wherever possible !
+100
--
Manik Surtani
Lead, JBoss Cache
manik(a)jboss.org
5985
days inactive
5985
days old
jbosscache-dev@lists.jboss.org
4 comments
2 participants
participants (2)
-
Bela Ban -
Manik Surtani