0

I am experiencing unexpected performance degradation when using vectors with classes.

I have a csv_File class that reads in a csv file and stores the contents in a 2D vector. There's a member function that allows access, e.g.

csv_File file("file.csv");
        file.access(2,2);

To access the 2,2 element.

Then, I have another class csv_Array that stores multiple csv_File objects in a vector, e.g private member vector<csv_File>
There's a member function that allows access, i.e. it returns a csv_File object, for example:

csv_Array file_array(5); //store 5 csv_File objects
        file_array.grab(0).access(2,2);

In the second line, grab returns a csv_File object (in this case, the first one) and access is a member function of the csv_File object.

However, I have noticed that the call
csv_Array.grab(0).access(2,2);
is much slower than it should be (it should be just 3 vector::at calls).

ADDITIONAL DETAILS (if necessary):
The application of this code is to load a bunch of csv files into memory before passing it to a friend class that will do some calculations with the data. Schematically, I have the following:

1) csv_Array has private member vector<csv_File> storage;
2) csv_Analysis is a class that is a friend of csv_Array
3) csv_Analysis accesses vector<csv_File> storage, which is in csv_Array
4) This access is done by passing csv_Analysis a reference to storage in csv_Array (so no copy hopefully....), e.g
public:
csv_Analysis(csv_Array &csv_block);

Thus, the call given above[file_array.grab(0).access(2,2); ]
actually has one additional class "level" in between and is more like

csv_Analysis analysis_Object(file_array); 
        analysis_Object.grab(0).access(2,2);

where grab acts in the same way, and is also defined as a member function of csv_Analysis class.

Edited by nocloud: n/a

6
Contributors
13
Replies
14
Views
5 Years
Discussion Span
Last Post by mike_2000_17
0

Yes, I think I should also include the relevant function prototypes, they are

//Access in csv_File
    std::string access(int row, int column);

    //grab in csv_Array and csv_Analysis
    csv_File grab(int index);

Edited by nocloud: n/a

0

You should be returning by reference not by value

std::string & access(int row, int column);

csv_File & grab(int index);
0

You should be returning by reference not by value

std::string & access(int row, int column);

csv_File & grab(int index);

And also provide a const reference version

0

Can you provide some details on how to make it a const reference? I tried changing the returns to references, e.g string& instead of string, but this gives a compiler error about invalid initialization of non-const reference...

0

Try the following:

// return a non-cost reference
    std::string & access(int row, int column);
    // returns a const reference, second const indicates that the method
    // doesn't alter the grid-instance that you're accessing from
    const std::string & access(int row, int column) const;

    ...

    csv_File & grab(int index);
    const csv_File & grab(int index) const;

If you're still getting compiler errors, provide more of the source code and specify which line is generating the error.

0

Thanks, this seems to mostly get the code working, however, I am still getting an error from another member function that I haven't listed here.

The call that throws the error is

vector<int> time;
current_data.grab(0).dump2ivector(1,time);

dump2ivector is declared as:

void csv_File::dump2ivector(int column, vector<int> &data) {....}

where {....} has inside of it a line like

data.push_back(atoi(access(0,0).c_str()));

Is there some problem with passing in a reference to a member function? That is the only thing I see special about dump2ivector

More specifically, the error is

error: no matching function call to csv_File::dump2ivector(int, std::vector<int. std::allocator<int> >&) const

Edited by nocloud: n/a

0

You need to mark the dump2ivector function as const, like so:

void csv_File::dump2ivector(int column, vector<int> &data) const {....} //notice const at the end.
0

You need to add const qualifiers in your function's declaration and definition.

//...

class csv_File
{
    //...

    void dump2ivector(int column, vector<int> &data) const;

    //...
};

//...

void csv_File::dump2ivector(int column, vector<int> &data) const { /*...*/ }

//...

EDIT: Too slow...

Edited by m4ster_r0shi: n/a

0

Thanks, this seems to mostly get the code working, however, I am still getting an error from another member function that I haven't listed here.

The call that throws the error is

vector<int> time;
current_data.grab(0).dump2ivector(1,time);

dump2ivector is declared as:

void csv_File::dump2ivector(int column, vector<int> &data) {....}

where {....} has inside of it a line like

data.push_back(atoi(access(0,0).c_str()));

Is there some problem with passing in a reference to a member function? That is the only thing I see special about dump2ivector

More specifically, the error is

error: no matching function call to csv_File::dump2ivector(int, std::vector<int. std::allocator<int> >&) const

Also don't name it time, because that might cause a name clash. Call it something like timeList or something.

0

Adding const did the trick. Can somebody explain why that const is required on the other member functions as well?

0

The const at the end of a member function means that the member function will not and cannot modify the state of the object on which the member function is called on.

A member function has a hidden parameter, the "this" pointer which points to the object on which the member function is called. In that sense, you could picture it like this:

struct Foo {
  int value;
  void Bar() {
    std::cout << this->value << std::endl;
  };
};

int main() {
  Foo f;
  f.value = 42;
  f.Bar();
};

//the above could be considered equivalent to:
struct Foo {
  int value;
};

void Foo_Bar(Foo* this) {
  std::cout << this->value << std::endl;
};

int main() {
  Foo f;
  f.value = 10;
  Foo_Bar(&f);
};

With a const at the end of the member function declaration, the above becomes:

struct Foo {
  int value;
  void Bar() const {
    std::cout << this->value << std::endl;
  };
};

int main() {
  Foo f;
  f.value = 42;
  f.Bar();
};

//the above could be considered equivalent to:
struct Foo {
  int value;
};

void Foo_Bar(const Foo* this) { //notice that "this" is now a pointer to const Foo object
  std::cout << this->value << std::endl;
};

int main() {
  Foo f;
  f.value = 10;
  Foo_Bar(&f);
};

That is the picture to keep in mind. cv-qualifiers at the end of the member function declarations are the cv-qualifiers of the object to which the 'this' pointer points to.

This was necessary in your case because once you have a const object, you can only call const member functions on it.

Edited by mike_2000_17: n/a

This question has already been answered. Start a new discussion instead.
Have something to contribute to this discussion? Please be thoughtful, detailed and courteous, and be sure to adhere to our posting rules.