[infinispan-dev] HotRodMarshaller interface

[Manik Surtani]

Fair point.

On 14 Jul 2010, at 07:08, Galder Zamarreño wrote:

> Manik,
> 
> WRT StreamingMarshaller in 4.1.x, shouldn't this method live in Marshaller instead?
> 
> ByteBuffer objectToBuffer(Object o) throws IOException;
> 
> Cheers,
> 
> On Jul 9, 2010, at 12:43 PM, Mircea Markus wrote:
> 
>> 
>> On 8 Jul 2010, at 14:57, Manik Surtani wrote:
>> 
>>> Guys,
>>> 
>>> I just created ISPN-532 (HotRodMarshaller needs cleaning up) targeted for Radegast CR2 and Final.  I know it is late in the day to be making API changes in an interface, but I would much rather this is done right prior to being baked into Final and hence eternity.  :)
>>> 
>>> 	http://fisheye.jboss.org/browse/Infinispan/trunk/client/hotrod-client/src/main/java/org/infinispan/client/hotrod/HotRodMarshaller.java?r=1850
>>> 
>>> So I have 3 issues to address with this interface.
>>> 
>>> 1.  Methods on the interface
>>> 
>>> The purpose of this is simple: the 2 methods, readObject() and marshallObject() are meant to perform diametrically opposed tasks (byte[] -> object in one case, object -> byte[] in the other) but the names are far from that.
>>> 
>>> I propose either of the following:
>>> 
>>> 	1) readObject() and writeObject()
>>> 	2) Simply, read() and write()
>>> 	3) Or if we want to follow org.infinispan.marshall.Marshaller conventions, objectFromByteBuffer(), objectToByteBuffer()
>>> 
>>> My preference would be (3).
>>> 
>>> 
>>> 2.  Interface name and package
>>> 
>>> Further, any reason why we call this HotRodMarshaller?  Isn't it just a marshaller that serializes arbitrary objects <--> byte[] and should contain no HotRod-specific logic?  I can see why we did not use the org.infinispan.marshall.Marshaller interface for this (too complex?) but perhaps we could create a org.infinispan.marshall.BasicMarshaller which just contains objectFromByteBuffer() and objectToByteBuffer(), and Marshaller can extend BasicMarshaller for Infinispan's internals and HotRod clients can directly implement BasicMarshaller?
>>> 
>>> This would allow people who write custom marshallers to reuse them for both the HotRod client as well as for Infinispan's internal marshalling.
>> +1
>> We can reuse the optimised marshalling logic from core this way, so that makes a lot of sense.
>> I'm thinking a better name would be ByteArrayMarshaller/BitesMarshaller (vs BasicMarshaller) - but obviously I'm not good with the names (see 1).
>>> 
>>> 3.  isKeyHint parameter on marshallObject()
>>> 
>>> What is this for?  
>> Here is the kind of application that would benefit from this: small keys with large values (e.g. an mp3 files kept in the grid, where file name is the key and the file is the value, or the gridFS). 
>> You are allowed to specify an default size for ExposedByteArrayOutputStream's underlaying byte[]. When this is exceeded the byte[] "doubles".
>> So for the above example specifying an 128bytes default value would be optimal for key serialization, but rather bad for values serialization: underlying byte[] would be created several times.
>> Also setting an value of e.g. 4MB for the default value would be not good for the keys, for same reasons.
>>> And surely there is a more elegant/generic way to achieve this?  
>> Perhaps overloading objectToByteBuffer(byte[] estimatedSize) ?
>>> 
>>> Cheers
>>> Manik
>>> 
>>> --
>>> Manik Surtani
>>> manik at jboss.org
>>> Lead, Infinispan
>>> Lead, JBoss Cache
>>> http://www.infinispan.org
>>> http://www.jbosscache.org
>>> 
>>> 
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> 
>> 
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> 
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
> 
> 
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
manik at jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org