Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve performance of removerepeatedpoints(multipoint) #609

Conversation

kalenikaliaksandr
Copy link
Contributor

No description provided.

POINT2D ptj, pti;
getPoint2d_p(mpt->geoms[i]->point, 0, &pti);
getPoint2d_p(mpt->geoms[j]->point, 0, &ptj);
if (distance2d_sqr_pt_pt(&ptj, &pti) > tolsq)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not enough - for instance, in the square in the center of this image there can possibly be three points within the distance.
image

Your method can still be a special case for tolerance=0, just plug back in old one :)

I see three big options how to implement this:

I. via double grid snap:

  • snap to grid of size = tolerance
  • check all the ones in the same bucket if they are the same
  • snap to grid of size = tolerance with a shift of 0.5
  • check all the ones in the same bucket if they are the same

II. double-sorting with window tracking:

  • order by X
  • iterate through list, look for duplicates back in the list until X is within tolerance
  • order by Y
  • iterate through list, look for duplicates back in the list until Y is within tolerance

III. tree

  • build a tree
  • while building a tree insertions, skip inserting points that have a point in tree within radius.

lwgeom_calculate_gbox((LWGEOM *)p2, &b2);

uint64_t h1, h2;
h1 = gbox_get_sortable_hash(&b1, 0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will probably be neater to refactor it to also allow points (it casts box back into a point internally and that piece can be isolated), and pass original srid

liblwgeom/lwgeom.c Outdated Show resolved Hide resolved
@kalenikaliaksandr kalenikaliaksandr marked this pull request as draft April 19, 2021 03:49
@kalenikaliaksandr kalenikaliaksandr force-pushed the improve-perf-of-dedup-multipoint branch 2 times, most recently from 3aedad7 to cccf3b3 Compare April 19, 2021 20:23
liblwgeom/lwgeom.c Outdated Show resolved Hide resolved
liblwgeom/lwgeom.c Outdated Show resolved Hide resolved
liblwgeom/lwgeom.c Outdated Show resolved Hide resolved
liblwgeom/lwgeom.c Outdated Show resolved Hide resolved
@kalenikaliaksandr kalenikaliaksandr force-pushed the improve-perf-of-dedup-multipoint branch 3 times, most recently from 633fed6 to 1f31351 Compare April 22, 2021 17:35
@kalenikaliaksandr kalenikaliaksandr marked this pull request as ready for review April 22, 2021 18:00
@Komzpa
Copy link
Member

Komzpa commented Jun 6, 2021

merged 423b519

@Komzpa Komzpa closed this Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants