Changes in JOSM core (referrers)

[Jiri Klement]

Hi,

I did some changes in JOSM core, that might bring some tickets in
following days.

First of all, Dataset now have map of primitives, that makes searching
for primitive by id much faster. It's used by addPrimitive method to
throw an exception if somebody is trying to add the same primitive
twice to the dataset (see r2398 for problem discoverd by this check),
removePrimitive prints warning if somebody tries to remove nonexisting
primitive. Also selection code was changed

Another thing is reference to dataset in OsmPrimitive. This reference
is set by Dataset.addPrimitive() method and reset by
Dataset.removePrimitive. If primitive is part of more than one dataset
then DataIntegrityProblemException is thrown. That was cause for
#3863.

Having reference to dataset in OsmPrimitive allowed new feature -
referrers (list of primitives that reference another primitive, for
example Way referrening nodes). For referrers to work reliably it is
necessary for every primitive to be part of exactly one dataset.
That's not true in JOSM so I've used a workaround - every primitive
have list of all referrers but method getReferrers() return only those
primitives that are part of the same dataset.

This workaround has one annoying consequence - it might cause memory
leaks. For example in following code:
Way wtemp = new Way(existingWay)
wtemp is temporary way, existingWay is some way in the dataset. wtemp
will be added as referrer to existingWay nodes and never released by
GC. I'm not sure yet if it's considerable and needs to be fixed (for
example by using SoftReference for primitives that don't have Dataset)

--
Jiri

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

[Frederik Ramm]

Hi,

Jiri Klement wrote:
> wtemp is temporary way, existingWay is some way in the dataset. wtemp
> will be added as referrer to existingWay nodes and never released by
> GC. I'm not sure yet if it's considerable and needs to be fixed (for
> example by using SoftReference for primitives that don't have Dataset)

I don't know what SoftReference is, but can't you use something like a
WeakHashMap for this?

Bye
Frederik

--
Frederik Ramm  ##  eMail frederik@...  ##  N49°00'09" E008°23'33"

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

[Jiri Klement]

On Sun, Nov 8, 2009 at 4:42 PM, Frederik Ramm <frederik@...> wrote:
> Jiri Klement wrote:
>>
>> wtemp is temporary way, existingWay is some way in the dataset. wtemp
>> will be added as referrer to existingWay nodes and never released by
>> GC. I'm not sure yet if it's considerable and needs to be fixed (for
>> example by using SoftReference for primitives that don't have Dataset)
>
> I don't know what SoftReference is, but can't you use something like a
> WeakHashMap for this?

Actually I meant WeakReference, not SoftReference. The only difference
is that SoftReference should be released only when there is no free
memory while WeakReference should be release as soon as there is no
other reference to the object

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

[Dave Hansen-3]

This changeset:

        http://josm.openstreetmap.de/changeset/2427/

removes the way_bbox_cache.  Unfortunately, this just *KILLS*
performance.  It makes some of my way search tests 10-20x slower.  We
use getBBox() really, really often and the CPU we use repeatedly
recreating them ends up being the bottleneck.

Could we put the way_bbox_cache back temporarily?  I don't think it was
hurting anything.

-- Dave


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

[Jiri Klement]

It's not possible to get it easily back as it was, but Way.getBBox()
should do the caching now. When I get time I'll add events to Dataset
so that Way will know when it's nodes were changed and update it's
bbox.

On Tue, Nov 10, 2009 at 6:33 PM, Dave Hansen <dave@...> wrote:

> This changeset:
>
>        http://josm.openstreetmap.de/changeset/2427/
>
> removes the way_bbox_cache.  Unfortunately, this just *KILLS*
> performance.  It makes some of my way search tests 10-20x slower.  We
> use getBBox() really, really often and the CPU we use repeatedly
> recreating them ends up being the bottleneck.
>
> Could we put the way_bbox_cache back temporarily?  I don't think it was
> hurting anything.
>
> -- Dave
>
>

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

[Dave Hansen-3]

On Tue, 2009-11-10 at 18:37 +0100, Jiri Klement wrote:
> It's not possible to get it easily back as it was, but Way.getBBox()
> should do the caching now.

I'm not sure it's implemented yet:

    @Override
    public BBox getBBox() {
        // TODO Precalculate way bbox (and update it every time nodes are moved or edited)
        return new BBox(this);
    }

but I'll go take a look.

BTW, thanks for all of the QuadBuckets cleanups, especially with the
generics around the Collection implementation.  You taught me a few
things. :)

-- Dave


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

[Dave Hansen-3]

On Tue, 2009-11-10 at 18:37 +0100, Jiri Klement wrote:
> It's not possible to get it easily back as it was, but Way.getBBox()
> should do the caching now. When I get time I'll add events to Dataset
> so that Way will know when it's nodes were changed and update it's
> bbox.

Just saw the commit.  Thanks for getting to this so quicky, Jiri!

-- Dave


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