Re: [josm-dev] Selection Performance
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
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
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
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
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
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
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
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