After thinking about it, I will try a revised approach to the JavaHL editor
API implementation.
First, is my understanding on how subversion works:
Objects involved:
* RA Session: svn_ra_session_t
* Editor: svn_delta_editor_t, edit_baton
* Directory: directory_baton
* File: file_baton
Memory management:
Pools are controlled by the caller. RA and Editor API do not require any
particular hierarchy of the pools. NOTE: This is the part that
I misunderstood before, thinking that I must use sub pools allocated from
the parent object pool.
Second, the design adjustments:
In my original design the the memory pools were hierarchical, so a logic
to handle cases where parent is disposed before child was necessary.
However, in the revised approach every object will have a subpool that will
be tied to the lifecycle of the object itself and will be allocated from
the JNI global memory pool. This eliminates the need to worry about
parent/child relationships and simplifies code. The outstanding question is
how to model java objects while being able to access svn_delta_editor_t
from within the JNI methods. Two designs come to mind:
1. All editor methods are in the ISVNEditor interface just like svn C API,
with ISVNFile and ISVNDirectory only wrapping baton and the object's memory
pool
2. Editor methods are split between ISVNEditor, ISVNFile and ISVNDirectory
interfaces but ISVNFile and ISVNDirectory implementations keep reference to
their ISVNEditor so that JNI code can use that reference to get to
the svn_delta_editor_t
Option #2 feels nicer, because file or directory object is sufficient to
perform object's operations without needing an explicit additional object
reference to the editor. This also eliminates case where editor is passed
file/directory objects created by another editor. So I will go with this
option for now.
Regards,
Vladimir
On Fri, May 25, 2012 at 4:47 PM, Vladimir Berezniker <vmpn_at_hitechman.com>wrote:
> Object hold their baton. Editor object also holds the editor driver (if I
> am understanding the terminology correctly).
>
> In detail:
>
> Editor holds reference to the editor and its baton plus parent/child
> object references:
>
> const svn_delta_editor_t * m_deltaEditor;
> void * m_editBaton;
> SVNRa & m_parentRaSession;
> CommitCallback * m_commitCallback;
>
> // Because of restriction #3 from svn_delta.h we only need to keep
> track of one directory
> SVNDirectory * m_rootDirectory;
>
> //But we do have to keep track of all files as delta's can be sent at
> the end
> std::set<SVNFile *> m_svnFiles;
>
> Directory holds its baton plus parent/child object references:
>
> void * m_dirBaton;
> SVNEditor & m_parentEditor;
> IDirectoryParent & m_parent;
> SVNDirectory * m_childDirectory;
>
> Vladimir
>
> On Fri, May 25, 2012 at 4:30 PM, Hyrum K Wright <hyrum.wright_at_wandisco.com
> > wrote:
>
>> Are you talking about maintaining the directory baton in the context
>> of something like an editor drive?
>>
>> -Hyrum
>>
>> On Fri, May 25, 2012 at 2:29 PM, Vladimir Berezniker <vmpn_at_hitechman.com>
>> wrote:
>> > The reason they are long lived is because they are reusable. E.g.
>> directory
>> > can be used for any number of files/directories within it. However,
>> > regarding not needing to keep track of objects I think I see where you
>> > are coming from. At the moment the parent and child objects are tied
>> because
>> > I allocated child pools from parent pools therefore their order of
>> cleanup
>> > is important. The reason for that design was that I could free up child
>> > resources when the parent was disposed even if the children were not.
>> But of
>> > the top of my head I cannot see why it has to be this way. For
>> example child
>> > object pools can be allocated from the global JNI pool instead
>> > thus eliminating the need to track children. Code will be simpler but
>> the
>> > JavaHL users won't be as protected from themselves.
>> >
>> > I need to dig through the code and think about it some more,
>> >
>> > Vladimir
>> >
>> >
>> > On Fri, May 25, 2012 at 2:36 PM, Hyrum K Wright <
>> hyrum.wright_at_wandisco.com>
>> > wrote:
>> >>
>> >> +1 to commit to trunk. Please include "Approved by: hwright" in the
>> >> log message.
>> >>
>> >> Out of curiosity, why do directory objects need to live across method
>> >> invocations with the RA interface? I would have thought that even if
>> >> this is true, it won't matter, since those objects would be pure Java
>> >> objects handled by the JVM.
>> >>
>> >> -Hyrum
>> >>
>> >> On Fri, May 25, 2012 at 12:35 PM, Vladimir Berezniker
>> >> <vmpn_at_hitechman.com> wrote:
>> >> > Greetings,
>> >> >
>> >> > Currently SVNBase class uses a member variable jthis to hold a copy
>> of
>> >> > jobject
>> >> > reference. This variable lives as long as SVNClient object lives,
>> >> > however,
>> >> > the
>> >> > reference itself is only valid during a single JNI call. To address
>> >> > this
>> >> > mismatch
>> >> > the attached patch passes the jobject reference as a parameter
>> instead.
>> >> > This
>> >> > eliminates the risk of the jobject reference being misused outside
>> the
>> >> > scope
>> >> > where it is valid. This mismatch becomes more evident on JavaHL
>> >> > RA editor API is implemented where objects like directories live
>> across
>> >> > method
>> >> > calls.
>> >> >
>> >> > [[[
>> >> > JavaHL: Explicitly pass jobject jthis when processing dispose() call
>> >> > rather
>> >> > than stashing a reference in the SVNBase class where it can be
>> misused
>> >> > later
>> >> >
>> >> > [ in subversion/bindings/javahl/native ]
>> >> >
>> >> > * SVNBase.cpp,
>> >> > SVNBase.h
>> >> > (dispose, jthis): Accept jobject jthis as explicit parameter to
>> >> > dispose()
>> >> > and
>> >> > delete the member variable jthis
>> >> >
>> >> > * SVNClient.cpp,
>> >> > SVNClient.h,
>> >> > SVNRepos.cpp,
>> >> > SVNRepos.h
>> >> > (dispose): Accept object jthis as explicit parameter and pass it to
>> >> > SVNBase::dispose
>> >> >
>> >> > * org_apache_subversion_javahl_SVNClient.cpp,
>> >> > org_apache_subversion_javahl_SVNRepos.cpp
>> >> > (Java_org_apache_subversion_javahl_SVNClient_dispose,
>> >> > Java_org_apache_subversion_javahl_SVNRepos_dispose):
>> >> > Pass object jthis as explicit parameter and pass it to the C++
>> >> > wrapper
>> >> > class
>> >> > ]]]
>> >> >
>> >> > Thank you,
>> >> >
>> >> > Vladimir
>> >>
>> >>
>> >>
>> >> --
>> >>
>> >> uberSVN: Apache Subversion Made Easy
>> >> http://www.uberSVN.com/
>> >
>> >
>>
>>
>>
>> --
>>
>> uberSVN: Apache Subversion Made Easy
>> http://www.uberSVN.com/
>>
>
>
Received on 2012-05-29 02:12:55 CEST