Hi,
On 16.5.12 9:40, Jukka Zitting wrote:
Hi,
On Mon, May 14, 2012 at 6:43 PM, Michael Dürig<[email protected]> wrote:
We had similar discussions before [1] and it keeps popping up all over
again: The NodeStore interface and friends all operate on NodeState and as I
said before, I don't think there is a reasonable way to implement this.
See my last two commits to .oak.plugins.memory for a rough in-memory
outline of how I envisioned the NodeStore mechanism working.
Yes I took a look at these already. This looks very similar to my very
first approach in the sandbox last year (the stuff was called NodeDelta
back then).
The way I see it, the NodeStateBuilder (NSB) is a pretty simple
construct. All it ever needs to keep track is the set of changes to be
applied to a single base NodeState. The context, path and parent
information in the current KernelNodeState is in my view not needed.
And even further, the implementation of NSB.getNodeState() by calling
context.getNodeState(getPath()) goes against the (in my view useful)
concept of separate NSB instances being completely independent, each
being responsible only for tracking the changes made against that
specific instance.
I see where you're going by making the NSB context keep track of the
full set of transient changes. I had considered that to rather be a
concern of the TreeImpl class. Instead of a shared root builder, each
TreeImpl would keep a separate NodeStateBuilder instance for tracking
the set of transient changes that apply to just that TreeImpl
instance.
All very well but... ;-)
And I'd use NodeStore.compare() for constructing the JSON diff to be
passed down to the MicroKernel. Since the NodeStore implementation
knows the implementation class of most of the involved NodeState
instances, it can even construct things like move and copy
instructions in the NodeStateDiff handler.
... I don't think this will reasonably work. There is just not enough
information in the raw states to reconstruct a JSON diff that also
includes the move operations. In particular, the information which is
missing is the order of the original operations.
After all this is the reason why I chose to track the transient changes
separately. The problem with that approach (as we both noted) is where
to but this additional "global" state.
Michael
I think this is wrong and we should use more exact types for
NodeStore (and related) interfaces instead of throwing exceptions at
runtime.
We can look at reorganizing the method calls so that fewer class casts
are needed. But if the contracts are revised as described above I
don't think there'll be too many places where casting to
implementation classes is needed, and most of them will have
reasonable (though potentially much slower) fallbacks to generic
types.
BR,
Jukka Zitting
