[josm-dev] Jumbo Patch

[Petr Nejedly]

Gabriel Ebner napsal(a):
> And these assumptions cannot be abstracted away by making all instance
> variables private.  You still have to make the assumption that you change a
> way by first cloning it, then modifying it and then adding a ChangeCommand to
> the undo list.  And this precise assumption is one that is broken by the patch
> in question.

I don't really care much about _that_ patch. My comment was general, in line
of what was my impression from the source code and stemming from my ideas how
would I speed up JOSM gradually (evolutionarily).

> As it stands, we cannot combine two commands properly, which makes it nearly
> impossible to build larger commands from smaller ones -- if you want to
> generate a sufficiently complex command, you'll have to do all sorts of
> low-level work.  The only sensible way to combine two commands would be to
> commit one to the undo list and then the other other -- but that would require
> the user to press undo twice.  If we wouldn't commit the first one first,

And there is a nice and elegant, long known way around this. Combining undo.
NetBeans editor use it for ages for combining trivial edits, otherwise you'd need
to undo every typed character separately.
You don't need a composite command as long as you can combine the undo operations
on the head of the undo queue:
Start a "transaction" - place a combiner on the undo queue.
Perform the first command, it will try to add itself on the undo queue,
combiner will fold it in.
Perform subsequent commands, they'll be folded in as well.
Finish the "transaction" - close the combiner - and any subsequent command
added to the undo queue will stand there on its own.
Undo through the combiner and it will perform the atomic undo of all the commands
it is composed from.

> changes to the same primitives in the second command would overwrite the ones
> from the first.
> 
> Of course I'd like the code to be as independent as possible from the concrete
> implementation, but that isn't a simple matter of wrapping all accesses to a
> public instance variable in getters and setters.
> 
>> So far all the changes I have in my sleeve (already coded) are nonintrusive,
>> but I think of few optimizations that would need update of unrelated parts,
>> and providing real scalability into JOSM would certainly require heavy
>> restructuralization, including wrapping some fields or some other measure
>> to cover external changes.
> 
> JOSM wasn't designed to work on large data sets.  It even stores the
> primitives in a linked list.  No index on the id.  No index on the coordinate.
> 
> Obviously it will take big changes to make it even remotely scalable, but
> these changes are not something to cover.  Plugins (especially the validator
> plugin) should have access to them too.
> 
>> I just want to know whether such changes will be accepted.
> 
> This particular patch has only generated such a heated discussion because it
> was so intrusive.
Well, I don't like the patch myself too. Not because it makes nodes private,
but because of the way it does so.

Still, if the behavior is defined by the Commands and not by the DataSet
and OsmPrimitives, the data in DataSet would better be accessible
only to the Commands. Otherwise the newcomers would think they should
operate on the DataSet data directly.

 > Nonintrusive patches have always been accepted so far,
> especially if they improve the performance.

Well, my patch (14.12.2007: Re: [josm-dev] Optimizations for larger data sets),
which both improves performance and is as nonintrusive as possible was still
not applied and we're having religious disputes instead...

-- 
Petr "Nenik" Nejedly, NetBeans/Sun Microsystems, http://www.netbeans.org
355/113 -- Not the famous irrational number PI, but an incredible simulation!