Skip to content

Prf maybe #6446

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Prf maybe #6446

wants to merge 2 commits into from

Conversation

tacaswell
Copy link
Member

Partially addresses #6444 at the cost of more memory usage.

The bottle neck seems to be the [][] calls.

With this patch the exaple in #6444 runs in 160ms so this does not fully address the issue.

Might also be worth looking at change the std:vector<bool> to std:vector<int> to see if that gets any performance gains?

This diff from @mdboom also gets comparable speed up at the cost of breaking the single point tests (which can be fixed)

diff --git a/src/.clang-format b/src/.clang-format
index d54ffce..de76f7f 100644
--- a/src/.clang-format
+++ b/src/.clang-format
@@ -1,4 +1,4 @@
----
+ ---
 # BasedOnStyle:  LLVM
 AccessModifierOffset: -2
 ConstructorInitializerIndentWidth: 4
diff --git a/src/_path.h b/src/_path.h
index 6b1b3d1..5554c12 100644
--- a/src/_path.h
+++ b/src/_path.h
@@ -112,7 +112,7 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins
         sy = vty0 = vty1 = y;

         for (i = 0; i < n; ++i) {
-            ty = points[i][1];
+            ty = points(i, 1);

             if (std::isfinite(ty)) {
                 // get test bit for above/below X axis
@@ -135,8 +135,8 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins
             }

             for (i = 0; i < n; ++i) {
-                tx = points[i][0];
-                ty = points[i][1];
+                tx = points(i, 0);
+                ty = points(i, 1);

                 if (!(std::isfinite(tx) && std::isfinite(ty))) {
                     continue;
@@ -183,8 +183,8 @@ void point_in_path_impl(PointArray &points, PathIterator &path, ResultArray &ins

         all_done = true;
         for (i = 0; i < n; ++i) {
-            tx = points[i][0];
-            ty = points[i][1];
+            tx = points(i, 0);
+            ty = points(i, 1);

             if (!(std::isfinite(tx) && std::isfinite(ty))) {
                 continue;
@@ -242,18 +242,18 @@ template <class PathIterator>
 inline bool point_in_path(
     double x, double y, const double r, PathIterator &path, agg::trans_affine &trans)
 {
-    std::vector<double> point;
-    std::vector<std::vector<double> > points;
-    point.push_back(x);
-    point.push_back(y);
-    points.push_back(point);
+    // std::vector<double> point;
+    // std::vector<std::vector<double> > points;
+    // point.push_back(x);
+    // point.push_back(y);
+    // points.push_back(point);

-    int result[1];
-    result[0] = 0;
+    // int result[1];
+    // result[0] = 0;

-    points_in_path(points, r, path, trans, result);
+    // points_in_path(points, r, path, trans, result);

-    return (bool)result[0];
+    // return (bool)result[0];
 }

 template <class PathIterator, class PointArray, class ResultArray>
@@ -285,18 +285,18 @@ template <class PathIterator>
 inline bool point_on_path(
     double x, double y, const double r, PathIterator &path, agg::trans_affine &trans)
 {
-    std::vector<double> point;
-    std::vector<std::vector<double> > points;
-    point.push_back(x);
-    point.push_back(y);
-    points.push_back(point);
+    // std::vector<double> point;
+    // std::vector<std::vector<double> > points;
+    // point.push_back(x);
+    // point.push_back(y);
+    // points.push_back(point);

-    int result[1];
-    result[0] = 0;
+    // int result[1];
+    // result[0] = 0;

-    points_on_path(points, r, path, trans, result);
+    // points_on_path(points, r, path, trans, result);

-    return (bool)result[0];
+    // return (bool)result[0];
 }

 struct extent_limits

@tacaswell tacaswell added this to the 2.1 (next point release) milestone May 19, 2016
@QuLogic QuLogic mentioned this pull request May 19, 2016
Closed
@mdboom
Copy link
Member

mdboom commented May 19, 2016

If the performance gains are similar, I think the proposed change to () is preferable since it doesn't involve allocating additional memory, and thus should scale better on larger data sizes.

@mdboom
Copy link
Member

mdboom commented May 19, 2016

I can probably find some time to put together the alternate proposal today.

@mdboom mdboom mentioned this pull request May 19, 2016
Merged
@tacaswell
Copy link
Member Author

Happily closing in favor of #6450

@tacaswell tacaswell closed this May 19, 2016
@QuLogic QuLogic modified the milestones: unassigned, 2.1 (next point release) May 19, 2016
@tacaswell tacaswell deleted the prf_maybe branch May 19, 2016 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tacaswell @mdboom @QuLogic