2

I've read a lot of question but no one answer me for my specific case.

Actually I have

std::vector<Point2Dd> points;
std::vector<Triangle> triangles;

Point2Dd is a class for a 2D point, it is not important to specify how it is implemented.

Triangle however is implemented like:

class Triangle{
    public:
     Triangle();
     Triangle(Point2Dd* p1, Point2Dd* p2, Point2Dd* p3);
     // Getter & setter

    private:
     Point2Dd* vA = nullptr;
     Point2Dd* vB = nullptr;
     Point2Dd* vC = nullptr;
}

that is, as three-pointers to vector of points.

Actually it work perfectly but I've think: if I add an other point into my vector and my vector change all memory address? All my triangles will be composed by invalid address.

I've read about using std::unique_ptr<Point2Dd> but I don't think is the best way.

Have you any solution? Thanks :)

--- EDIT 1 ---

To clarify my problem I explain what problem I'm trying to solve. I'm doing an incremental Delaunay Triangulation (no problem with that). So I have to add once by once a point and update my triangulation.

So I've think to manage triangle as a three pointer to my points. Also I have a dag (Node -> Triangles with three children) and a structure that save adjacent triangles.

This is why I've thinked to use always a pointer, so I don't have to copy in three different structures the same points.

This is why I need to solve this problem to prevent memory reallocation.

Aso Strife
  • 1,089
  • 3
  • 12
  • 31

3 Answers3

6

Yes because after this I've a very heavy algorithm, so I need to optimize all as I can..

In this case, start with copies of data.

struct Triangle{
     Triangle();
     Triangle(Point2Dd p1, Point2Dd p2, Point2Dd p3);
     // Getter & setter

    private:
     Point2Dd vA, vB, vC;
};

Although measurement is the only way to know for sure, the loss of cache locality and the indirect memory access inherent in a pointer-based solution is almost certain to result in run times an order of magnitude slower.

Richard Hodges
  • 68,278
  • 7
  • 90
  • 142
  • But copying object is not very expansive? – Aso Strife Aug 15 '17 at 10:26
  • 3
    @AsoStrife copying a double isnt any slower than copying a pointer – 463035818_is_not_an_ai Aug 15 '17 at 10:27
  • 2
    @AsoStrife making 1 copy is far quicker than indirecting a million times... – Richard Hodges Aug 15 '17 at 10:28
  • 4
    @AsoStrife To emphasize again: You are not understanding optimization correctly at all. Here is what you should do for starters: Write for correctness and clarity first, then test if it's fast enough. If it's not, find the current bottleneck(s) with a profiler and optimize those until the program runs sufficiently fast. Making random guesses about performance will most certainly end you up with slow and incorrect or at least unmaintainable code. – Baum mit Augen Aug 15 '17 at 10:33
  • Ok thanks! I could try this solution or the other you suggested: "You can save indices instead of pointers plus a pointer or reference to the original vector. Those will always stay valid" – Aso Strife Aug 15 '17 at 10:38
  • But I was think: If I do for example: triangle.getVertexA() I'll get the index not the point... So in other class I can't use the coordinates of point if I don't pass the vector of points... – Aso Strife Aug 15 '17 at 10:54
  • 1
    @AsoStrife ...copy once... indirect never...it's the fastest way... Copying blocks of data is far more efficient that indirect access on a modern memory system. RAM is not really Random Access anymore - it's accessed in bursts of sequential access. That is hidden by the processor data cache - *if* you keep your data together. I know it's counter-intuitive, but it's true. – Richard Hodges Aug 15 '17 at 11:20
2

Two simple options:

  1. Use an std::vector<Point*>. Fill it with new and empty it with delete. Your points will live on the heap and they won't be invalidated when the vector grows.

  2. Call std::vector::reserve with size n before you push_back the points. reserve allocates space for the vector's data, so addresses won't be invalidated unless you push_back more than npoints. std::vector::resize could also work but check documentation first as it's slightly different.

Edit:

  1. A commenter mentioned saving indices which is a great idea and probably simpler than all of this.
sudo rm -rf slash
  • 1,156
  • 2
  • 16
  • 27
  • So if I use `std::vector points` I can user `point.push_back(p)` without problem? The elements will never be reallocated? – Aso Strife Aug 15 '17 at 15:32
  • @AsoStrife - It's possible that the element will be reallocated (improbable if you use `reserve()` with an adequate value) but, in this case, the elements are pointers; so the reallocation doesn't change the value of the pointers. – max66 Aug 15 '17 at 17:36
  • @AsoStrife you can do `points.push_back(new Point)`. I can't tell if the `p` in your comment refers to a value or a pointer. And the pointers will stay valid but may be moved around even without resize (as I think @max66 said). If you're explicit with the types in your code, the compiler should catch most errors in this implementation E.g. `points.push_back()` will be an error since it expect a pointer to a point. – sudo rm -rf slash Aug 15 '17 at 21:29
  • I've solve the problem using `std;;vectir points`. Vector will be reallocate, but the pointers are still valid. – Aso Strife Aug 16 '17 at 10:36
0

I've read about using std::unique_ptr but I don't think is the best way

And what about shared pointers ?

I mean... if your Triangle class contains three std::shared_ptr<Point2Dd>, instead of three old style pointers, and if points is a std::vector<std::shared_ptr<Point2Dd>> instead of a vector of Points2Dd, you should able to write something as the following example

#include <memory>
#include <vector>

struct Point2Dd
 { };

class Triangle
 {
   private:
      using sp2Dd = std::shared_ptr<Point2Dd>;

      sp2Dd vA, vB, vC;

   public:
      Triangle()
       { }

      Triangle (sp2Dd const & p1, sp2Dd const & p2, sp2Dd const & p3)
         : vA{p1}, vB{p2}, vC{p3}
       { }
 };

int main ()
 {
   std::vector<std::shared_ptr<Point2Dd>> points;
   std::vector<Triangle>                  triangles;

   points.emplace_back(new Point2Dd{}); // or using std::make_shared(),
   points.emplace_back(new Point2Dd{}); // if C++14 is available
   points.emplace_back(new Point2Dd{});

   triangles.emplace_back(points[0U], points[1U], points[2U]);
 }

p.s.: if isn't important that all the element of points are stored in a unique sequential area, I suggest you to consider the use of std::deque instead of std::vector

max66
  • 65,235
  • 10
  • 71
  • 111
  • I've tried to create a `std::vector>` but when I try to compile I recive this: `no match for ‘operator==’ (operand types are ‘std::shared_ptr >’ and ‘const Point2D’) { return *__it == _M_value; }`. Using `points.emplace_back(new Point2Dd{x,y})` `` – Aso Strife Aug 15 '17 at 18:42
  • @AsoStrife - you should show us a minimal example that reproduce the problem; anyway, I suppose you have something like `points[0] == Point2Dd{};`. In my example `points[0]` isn't a `Point2Dd` anymore; now it's a **pointer** (wrapped in a class, but a pointer) to a `Point2Dd`; so you have to dereference it to compare it with another pointer; something like `*points[0] == Point2Dd{};` – max66 Aug 15 '17 at 18:51