Re: [josm-dev] Selection Performance

2009-10-09 Thread Dave Hansen
On Fri, 2009-10-09 at 07:47 +0200, Karl Guggisberg wrote:
  So I would go even further and deprecate/remove the selection notion from 
  the primitive altogether. It is used in few places only anyway (beyond 
  painting) and would be well served by a selection set on the Layer anyway.
 
 I support that.

Here's a patch that deprecates the OsmPrimitive selection methods:

http://josm.openstreetmap.de/ticket/3676

Once that's in, I'll put together a patch to keep selection state
entirely within the DataSet.

-- Dave


___
josm-dev mailing list
josm-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/josm-dev


[josm-dev] Selection Performance

2009-10-08 Thread Dave Hansen
It takes almost a second for me to do a mouse click and have a single
object be selected.  I think it's worse for large data sets, but it
seems to exist most of the time despite the size of the data set.
Personally, I think a whole second to wait for a mouse click is pretty
bad.

Most of the time appears to be spent notifying things about the
selection change.  I just counted 110 of these, and we fire once for the
clearSelection() and again for the new selection.  They all seem to add
up to almost a second of latency.  I actually went and printed out a
bunch of timestamps to confirm this.

Any thoughts on how to make this better?  I've been using my QuadBuckets
code to make the click coordinate searches faster, but I'm clueless as
to what to do with these notifiers.

DataSet.fireSelectionChanged() 1254985384285 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@3082f392
DataSet.fireSelectionChanged() 1254985384285 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@65450f1f
DataSet.fireSelectionChanged() 1254985384285 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@18e3f02a
DataSet.fireSelectionChanged() 1254985384285 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@1cac622a
DataSet.fireSelectionChanged() 1254985384285 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@7960c21a
DataSet.fireSelectionChanged() 1254985384285 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@3f677737
DataSet.fireSelectionChanged() 1254985384285 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@21c3dc66
DataSet.fireSelectionChanged() 1254985384285 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@41babddb
DataSet.fireSelectionChanged() 1254985384295 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@4b069693
DataSet.fireSelectionChanged() 1254985384295 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@1d87b360
DataSet.fireSelectionChanged() 1254985384296 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@381172c5
DataSet.fireSelectionChanged() 1254985384355 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@1860045
DataSet.fireSelectionChanged() 1254985384355 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@47bb2cb
DataSet.fireSelectionChanged() 1254985384355 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@55172fb9
DataSet.fireSelectionChanged() 1254985384355 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@653e4653
DataSet.fireSelectionChanged() 1254985384355 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@5b7b2712
DataSet.fireSelectionChanged() 1254985384355 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@72423da9
DataSet.fireSelectionChanged() 1254985384366 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@5ade5cd9
DataSet.fireSelectionChanged() 1254985384366 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@3ca1d92a
DataSet.fireSelectionChanged() 1254985384378 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@70a6aa31
DataSet.fireSelectionChanged() 1254985384393 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@4defb0be
DataSet.fireSelectionChanged() 1254985384404 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@333cb1eb
DataSet.fireSelectionChanged() 1254985384404 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@55c4d594
DataSet.fireSelectionChanged() 1254985384415 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@6da21389
DataSet.fireSelectionChanged() 1254985384415 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@2bb0bf9a
DataSet.fireSelectionChanged() 1254985384415 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@60f32dde
DataSet.fireSelectionChanged() 1254985384415 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@7d487b8b
DataSet.fireSelectionChanged() 1254985384415 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@21a722ef
DataSet.fireSelectionChanged() 1254985384415 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@63e68a2b
DataSet.fireSelectionChanged() 1254985384426 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@46bd530
DataSet.fireSelectionChanged() 1254985384437 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@43794494
DataSet.fireSelectionChanged() 1254985384447 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@4e857327
DataSet.fireSelectionChanged() 1254985384459 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@1b4b2db7
DataSet.fireSelectionChanged() 1254985384475 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@64a65760
DataSet.fireSelectionChanged() 1254985384486 
org.openstreetmap.josm.actions.josmaction$selectionchangeadap...@64b2933f

Re: [josm-dev] Selection Performance

2009-10-08 Thread Dave Hansen
On Thu, 2009-10-08 at 00:10 -0700, Dave Hansen wrote:
 It takes almost a second for me to do a mouse click and have a single
 object be selected.  I think it's worse for large data sets, but it
 seems to exist most of the time despite the size of the data set.
 Personally, I think a whole second to wait for a mouse click is pretty
 bad.
 
 Most of the time appears to be spent notifying things about the
 selection change.  I just counted 110 of these, and we fire once for the
 clearSelection() and again for the new selection.  They all seem to add
 up to almost a second of latency.  I actually went and printed out a
 bunch of timestamps to confirm this.
 
 Any thoughts on how to make this better?  I've been using my QuadBuckets
 code to make the click coordinate searches faster, but I'm clueless as
 to what to do with these notifiers.

Digging in a bit more... Virtually none of these actions care about the
contents of selections.  The vast majority just do:

@Override
protected void updateEnabledState() {
setEnabled(getCurrentDataSet() != null  ! 
getCurrentDataSet().getSelected().isEmpty());
}

The problem is that getSelected() iterates over the entire data set
looked for selected OsmPrimitives.  We do this ~112 times for each mouse
click.  That's a bit, um, suboptimal.

I tried a really quick and dirty solution which is to just keep a copy
of the selected primitives around.  That makes a *huge* difference.
Maybe we should have a heuristic where a selection that's less than 10%
(or whatever) of the size of the data set gets cached.  Any thoughts?

+HashSetOsmPrimitive selectedPrimitives = new HashSetOsmPrimitive();
+
 public void setSelected(Collection? extends OsmPrimitive selection) {
-clearSelection(nodes);
-clearSelection(ways);
-clearSelection(relations);
+clearSelection(selectedPrimitives);
+selectedPrimitives.clear();
 for (OsmPrimitive osm : selection) {
 osm.setSelected(true);
 }
+selectedPrimitives.addAll(selection);
 fireSelectionChanged(selection);
 }
 public CollectionOsmPrimitive getSelected() {
+return selectedPrimitives;
 }

-- Dave


___
josm-dev mailing list
josm-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/josm-dev


Re: [josm-dev] Selection Performance

2009-10-08 Thread Petr Nejedlý
Dave Hansen napsal(a):
 On Thu, 2009-10-08 at 00:10 -0700, Dave Hansen wrote:
 It takes almost a second for me to do a mouse click and have a single
 object be selected.  I think it's worse for large data sets, but it
 seems to exist most of the time despite the size of the data set.
 Personally, I think a whole second to wait for a mouse click is pretty
 bad.

 Most of the time appears to be spent notifying things about the
 selection change.  I just counted 110 of these, and we fire once for the
 clearSelection() and again for the new selection.  They all seem to add
 up to almost a second of latency.  I actually went and printed out a
 bunch of timestamps to confirm this.

 Any thoughts on how to make this better?  I've been using my QuadBuckets
 code to make the click coordinate searches faster, but I'm clueless as
 to what to do with these notifiers.
 
 Digging in a bit more... Virtually none of these actions care about the
 contents of selections.  The vast majority just do:
 
 @Override
 protected void updateEnabledState() {
 setEnabled(getCurrentDataSet() != null  ! 
 getCurrentDataSet().getSelected().isEmpty());
 }
 
 The problem is that getSelected() iterates over the entire data set
 looked for selected OsmPrimitives.  We do this ~112 times for each mouse
 click.  That's a bit, um, suboptimal.
 
 I tried a really quick and dirty solution which is to just keep a copy
 of the selected primitives around.  That makes a *huge* difference.

First and foremost, selection status is not a property of the data.
Changing the data set just because the user have selected something is 
plain wrong. Having a per-dataset selection is quite disturbing too - 
there should be only one, system-wide.

Regards,
   Nenik.

___
josm-dev mailing list
josm-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/josm-dev


Re: [josm-dev] Selection Performance

2009-10-08 Thread Dave Hansen
On Thu, 2009-10-08 at 18:35 +0200, Karl Guggisberg wrote:
 1. to migrate to node.setSelected(..) and way.isSelected()
 2. to change the behaviour behind
 OsmPrimitive:setSelected()/isSelected(),
 possibly using a reference from
the primitive to it's parent dataset  (see recent dicussion about
 backreference in this list)

I think I missed this discussion.  Do you have any pointers to it?  A
quick nabble search also didn't turn anything up.

-- Dave


___
josm-dev mailing list
josm-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/josm-dev


Re: [josm-dev] Selection Performance

2009-10-08 Thread Karl Guggisberg
I was referring to the discusion about backreferences and referreres
http://lists.openstreetmap.org/pipermail/josm-dev/2009-October/003435.html

-- Karl

-Ursprüngliche Nachricht-
Von: Dave Hansen [mailto:d...@sr71.net] 
Gesendet: Donnerstag, 8. Oktober 2009 18:45
An: karl.guggisb...@guggis.ch
Cc: 'Petr Nejedlý'; 'josm-dev'
Betreff: Re: AW: [josm-dev] Selection Performance

On Thu, 2009-10-08 at 18:35 +0200, Karl Guggisberg wrote:
 1. to migrate to node.setSelected(..) and way.isSelected() 2. to 
 change the behaviour behind OsmPrimitive:setSelected()/isSelected(),
 possibly using a reference from
the primitive to it's parent dataset  (see recent dicussion about 
 backreference in this list)

I think I missed this discussion.  Do you have any pointers to it?  A quick
nabble search also didn't turn anything up.

-- Dave


___
josm-dev mailing list
josm-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/josm-dev


Re: [josm-dev] Selection Performance

2009-10-08 Thread Petr Nejedlý
Dave Hansen napsal(a):
  On Thu, 2009-10-08 at 10:18 +0200, Petr Nejedlý wrote:
  First and foremost, selection status is not a property of the data.
  Changing the data set just because the user have selected something is
  plain wrong. Having a per-dataset selection is quite disturbing too -
  there should be only one, system-wide.
 
  OK, so you're saying that because selecting things is not a property of
  the data that it shouldn't be a part of the data set?  Uh, ok.  Aren't
  ways and nodes part of the data set?  Check out
  OsmPrimitive.setSelected().  Sure seems to *ME* like selections are part
  of the data set.
Selections are currently part of the OSMPrimitive API, sure.
What I'm saying is that it doesn't belong there.
In the worst case, it may be the property of the DataSet, but...

Karl Guggisberg napsal(a):
 Do you have a better suggestion on where to stick the selection cache
 or whatever we want to call it than in the DataSet?
 I think the selection cache (a collection of the currently selected
 primitives) should be maintained for each data layer (either by the layer or
 by some global selection cache which keeps a selection state per layer). If
 the dataset wasn't attached to a data layer why would you want to
 select/unselect objects? What I never liked is that there is a boolean field
 'selected' on OsmPrimitive - this looks weired.

... exactly! Layer is the view of the DataSet. No view, no selection.
Several views of the same dataset - maybe several different selections.
Asking a primitive whether is it selected or not is a strange concept 
anyway. Why should it know? In which context?
Imagine a tactical overview (a small view of the complete in-memory 
data rendered in the screen corner) or any other auxiliary view of the 
same dataset. You'd like to use the same rendering code, yet you don't 
want it to render the selected primitives the same way, for example.

So I would go even further and deprecate/remove the selection notion 
from the primitive altogether. It is used in few places only anyway 
(beyond painting) and would be well served by a selection set on
the Layer anyway.

Regards,
   Nenik

PS: In case you are afraid of the memory footprint of the selection set,
you can borrow the Storage class from josm-ng. It will cost around 7B
per selected entry (that is pretty small unless you select everything)
depending on the target load factor and allow you to remove a boolean 
from OsmPrimitive (costing anything between 0 and 4 bytes per 
OsmPrimitive in memory, depending on several factors). Many more places 
in JOSM would benefit from Storage anyway (String cache in OsmReader,
Long-OsmPrimitive maps in the OsmReader and the MergeVisitor, ...)

___
josm-dev mailing list
josm-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/josm-dev


Re: [josm-dev] Selection Performance

2009-10-08 Thread Karl Guggisberg
 So I would go even further and deprecate/remove the selection notion from the 
 primitive altogether. It is used in few places only anyway (beyond painting) 
 and would be well served by a selection set on the Layer anyway.

I support that.

-- Karl 

-Ursprüngliche Nachricht-
Von: Petr Nejedlý [mailto:p...@nejedli.cz] 
Gesendet: Donnerstag, 8. Oktober 2009 22:55
An: karl.guggisb...@guggis.ch
Cc: 'Dave Hansen'; 'josm-dev'
Betreff: Re: AW: [josm-dev] Selection Performance

Dave Hansen napsal(a):
  On Thu, 2009-10-08 at 10:18 +0200, Petr Nejedlý wrote:
  First and foremost, selection status is not a property of the data.
  Changing the data set just because the user have selected something is   
  plain wrong. Having a per-dataset selection is quite disturbing too -   
  there should be only one, system-wide.
 
  OK, so you're saying that because selecting things is not a property of   
  the data that it shouldn't be a part of the data set?  Uh, ok.  Aren't   
  ways and nodes part of the data set?  Check out   
  OsmPrimitive.setSelected().  Sure seems to *ME* like selections are part   
  of the data set.
Selections are currently part of the OSMPrimitive API, sure.
What I'm saying is that it doesn't belong there.
In the worst case, it may be the property of the DataSet, but...

Karl Guggisberg napsal(a):
 Do you have a better suggestion on where to stick the selection cache
 or whatever we want to call it than in the DataSet?
 I think the selection cache (a collection of the currently selected
 primitives) should be maintained for each data layer (either by the 
 layer or by some global selection cache which keeps a selection state 
 per layer). If the dataset wasn't attached to a data layer why would 
 you want to select/unselect objects? What I never liked is that there 
 is a boolean field 'selected' on OsmPrimitive - this looks weired.

... exactly! Layer is the view of the DataSet. No view, no selection.
Several views of the same dataset - maybe several different selections.
Asking a primitive whether is it selected or not is a strange concept anyway. 
Why should it know? In which context?
Imagine a tactical overview (a small view of the complete in-memory data 
rendered in the screen corner) or any other auxiliary view of the same dataset. 
You'd like to use the same rendering code, yet you don't want it to render the 
selected primitives the same way, for example.

So I would go even further and deprecate/remove the selection notion from the 
primitive altogether. It is used in few places only anyway (beyond painting) 
and would be well served by a selection set on the Layer anyway.

Regards,
   Nenik

PS: In case you are afraid of the memory footprint of the selection set, you 
can borrow the Storage class from josm-ng. It will cost around 7B per selected 
entry (that is pretty small unless you select everything) depending on the 
target load factor and allow you to remove a boolean from OsmPrimitive (costing 
anything between 0 and 4 bytes per OsmPrimitive in memory, depending on several 
factors). Many more places in JOSM would benefit from Storage anyway (String 
cache in OsmReader,
Long-OsmPrimitive maps in the OsmReader and the MergeVisitor, ...)


___
josm-dev mailing list
josm-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/josm-dev