[osmosis-dev] read-only entities

Jochen Topf jochen at remote.org
Sat Feb 7 09:16:55 GMT 2009


I thought about the read-only entitys some more and still dislike any solution
where we have extra classes for them. The only way I could imagine doing it
is with some kind of wrapper class for all entities that implements a
copy-on-write. But I can't think of a way to implement this without copying
too much code.

So here is my proposed solution:

The Entity class gets an attribute "boolean read_only". On creation it is
false, it can be set by calling entity.makeReadOnly(). All setter function
check this attribute and throw an exception if ready-only is set.  There is
also a method isReadOnly() to check this attribute and a cloneIfReadOnly()
which will return a clone of the entity (with the read-only attribute not set)
if it is read only and the entity itself if not.

The --tee task and all other tasks that somehow create threads have to either
create a deep copy of an Entity for each thread or use the same entity and call
.makeReadOnly() on it (preferred). That takes care of thread-safety.

Any task that knows that it will not change any entities it gets, simply calls 
getEntity() as before in the process() method of a task. It doesn't care whether
the read-only attribute is set or not.

If the tasks knows beforehand that it will probably change the entity, it calls
getEntity().cloneIfReadOnly() instead of just getEntity().

If the task only sometimes changes an entity it can still call getEntity() and
do the cloneIfReadOnly() at any convenient point when it has found out that
it will need to change this entity.

So, this hole process means that there is no copying penalty for the common
case. A copy will only be done if there is a --tee *and* some task after it
changing an entity. The penalty of creating a deep clone of an entity is only
ever incurred when it is really needed. Of course there is the added overhead
on checking one boolean every time a setter method is called. This is not
strictly necessary, because well-behaved tasks would never call a setter method
on a read-only entity, but of course it must be done in case there is a
misbehaving task.

Not much change in the current code is needed to implement this and it is easy
for writers of a task, they just have to know about the cloneIfReadOnly() and
call it if needed.

The only drawback I can see is that some compile time checks aren't possible
any more. The setter methods have to check at run time. But I think in this case
thats just a very minor drawback.

We can also throw in another goodie: One common way an entity might change is
if the tags change but nothing else. Or the nodes in a way change and nothing
else. So in addition to the cloneIfReadOnly() we could have methods that will
clone an entity but leave the tags or nodes lists empty or so. So we don't have
to have the overhead of deep cloning the entity and throwing half of it away
right after that. But we can figure out how to best do this later.

Jochen
-- 
Jochen Topf  jochen at remote.org  http://www.remote.org/jochen/  +49-721-388298





More information about the osmosis-dev mailing list