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

Add GeometryCollection dimension cache #1103

Merged

Conversation

dr-jts
Copy link
Contributor

@dr-jts dr-jts commented Dec 19, 2024

Heterogeneous (mixed) GeometryCollections have to use an O(n) algorithm to determine the dimensions of the elements. This causes performance issues in algorithms which use the dimension of the collection (see #1100).

This PR solves this problem by caching the computed dimensions of a GeometryCollection. It adds a single reference to each collection object, but the actual cache is lazily created when dimension information is accessed.

For example, for a CoverageUnion of a grid of 1M rectangles this improves the performance from 17 s to 370 ms (45x).

Fixes #1100

@dr-jts dr-jts merged commit 47b52bd into locationtech:master Dec 19, 2024
2 checks passed
@dr-jts dr-jts deleted the add-geometrycollection-dimension-cache branch December 19, 2024 21:15
}

public boolean hasDimension(int dim) {
if (dim == Dimension.A && hasA) return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the dim == x check here. If we have a mixed polygon/line collection, isn't dim equal to Dimension.A but hasDimension(Dimension.L) should be true?

Copy link
Contributor Author

@dr-jts dr-jts Dec 20, 2024

Choose a reason for hiding this comment

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

dim is the argument to hasDimension. So for hasDimension(Dimension.L) dim is 1, and that matches

dim == Dimension.L && hasL

Actually this logic would be simpler as:

    if (dim == Dimension.A) return hasA;
    if (dim == Dimension.L) return hasL;
    if (dim == Dimension.P) return hasP;
    return false;

@dbaston do you agree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or even better:

switch (dim) {
case Dimension.A: return hasA;
case Dimension.L: return hasL;
case Dimension.P: return hasP;
}
return false;

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, I was pretty sure I was having a brain failure of sorts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See 9133b47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CoverageUnion - improve performance for huge amount of geometries
2 participants