Hi Alan,
Apologies for the 2 week+ delay in replying -- I was away on holiday.
On 03/10/18 15:24, Alan Bateman wrote:
> On 03/10/2018 10:14, Andrew Dinn wrote:
>> :
>> Sure, I'd be happy to change this.
>>
>> Would READ_ONLY_SYNC and READ_WRITE_SYNC be suitable alternatives? Or do
>> you have something else in mind?
>>
> I think that works but it means looking at the proposed
> MappedByteBuffer::isPersistent too. MappedByteBuffer hasn't needed
> methods to test if the mapping is read-only, read-write or private
> mapping and I wonder if isPersistent is really needed.
That's a good question. I guess isPersistent is not really needed as a
private method since a field lookup would do. It is obviously of no use
as a protected method because MappedByteBuffer sits in a sandwich
between ByteBuffer and DirectByteBuffer and, hence, cannot be
specialized. So, why make it public?
The immediate intentions here is to use the new MappedByteBuffer API as
a base layer for implementation of a library of classes that provide
equivalent capabilities to the libs that extend Intel's libpmem,
providing various managed persistent data overlays on the raw NVM bytes
such as journals, block array stores etc. As far as that goal is
concerned there is arguably no need to provide isPersistent as a public
API because these client classes would normally only employ a buffer
mapped to NVM.
However, I am not sure that is always going to be the only desired mode
of operation. It may be, for example, that we want to use these classes
to operate over mapped files as well as mapped NVM. That's very likely
not going to give great performance (although in the case of a block
array store whose block sizes are file page multiples it might not make
that much difference). However, it does allow for compatibility mode
operation when NVM is not available. Even so, I believe those clients
will not actually care what type of buffer they use.
Other client classes might also need to be able to provide these two
alternative modes of operation -- where the underlying ByteBuffer may or
may not be persistent -- and it is not clear to me that they would not
care about whether the mapping was to NVM or to a conventional file. It
might be that some clients would want to use the buffer in different
ways depending on how it is mapped. Jonathan Halliday (in cc) actually
defined the method as public on the rather liberal assumption that this
might be how it was used but it seems he did not have a specific use
case in mind.
Of course, a client could always track this information itself. However,
since this datum is i) a property of the ByteBuffer and ii) already
stored in the ByteBuffer I felt it was best to expose the property via a
public getter -- arguably it ought to be final. If you think that is
inappropriate or would prefer to remove it so as to minimize the new API
surface I would be happy to follow your decision.
regards,
Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
