Processing XYZ data from a large file I'm new to programming (1.5 years experience at university), and I'm working on my first computer vision related project! My goal is to take a file which might have CSV (comma separated) or txt format with XYZ coordinates and process the data. I want to round the X and Y coordinates into integers and leave the Z coordinate as a floating point.

Each file ranges from 200000 to 1400000 lines of XYZ data is is formatted as so:

# information about .txt file
253.9999929 58.0428367 -21.3930063253
.9999929 59.0435773 -21.2499391255
etc.....


Do note that the file might have commas, like a CSV file.

The function that is my main bottleneck is:

Loadfile(string filename);


I'm pretty sure my bottleneck is from the ifstream for reading the file one line at a time, but I'm having trouble finding ways to improve it. One idea I had was to convert the file into an array of bytes and read split it up into different domains and give it to multiple threads to read concurrently, but I'm not sure if that's a good practice.

Here is the my code so far:

class XYZCoordinate{
public:
    int x;
    int y;
    float z;

    XYZCoordinate(int X, int Y, float Z){
    x = X;
    y = Y;
    z = Z;
    }
};

class Loadfileoutput
{
public:
    int xmin;
    int xmax;
    int ymin;
    int ymax;
    vector&lt;XYZCoordinate&gt; values;

    Loadfileoutput(int xmn, int xmx, int ymn, int ymx, vector&lt;XYZCoordinate&gt; val){
    xmin = xmn;
    xmax = xmx;
    ymin = ymn;
    ymax = ymx;
    values = val;
    }
};

Loadfileoutput Loadfile(string filename)
{
    std::ifstream in(filename);
    if (!in) {
        cout &lt;&lt; "Cannot open input file.\n";
        exit(EXIT_FAILURE);
    }

    std::string str;
    bool isfirst = true;
    int xmin, xmax, ymin, ymax;
    vector&lt;XYZCoordinate&gt; file = vector&lt;XYZCoordinate&gt;();
    while (std::getline(in, str)) //potential cause of bottleneck
    {
        if (str[0] == '#') //.skipping the .txt file header
            continue;
        vector&lt;string&gt; v = vector&lt;string&gt;();

        boost::split(v, str, boost::is_any_of(" ,"));

        string xstr = v[0];
        string ystr = v[1];
        string zstr = v[2];
        int xint, yint;
        float x,y,z;

        stringstream(v[0]) &gt;&gt; x;
        xint = (int)round(x);

        stringstream(v[1]) &gt;&gt; y;
        yint = (int)round(y);

        stringstream(v[2]) &gt;&gt; z;

        XYZCoordinate temp = XYZCoordinate(xint, yint, z);
        file.push_back(temp);

        //get bounds
        if (isfirst)
        {
            xmin = xint;
            xmax = xint;
            ymin = yint;
            ymax = yint;
            isfirst = false;
        }
        else
        {
            //set xmin/max for scaling
            if (xint &gt; xmax)
                xmax = xint;
            else if (xint &lt; xmin)
                xmin = xint;
            //set ymin/max for scaling
            if (yint &gt; ymax)
                ymax = yint;
            else if (yint &lt; ymin)
                ymin = yint;
        }
    }   
    Loadfileoutput output = Loadfileoutput(xmin,xmax,ymin,ymax,file);

    return output;
}
There are a number of things that may help you improve your program.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Use the required #includes

The code uses a number of #includes that are not listed.  It was not difficult to infer, but it helps reviewers if the code is complete.  After a bit of looking, it seems that this is what's needed:

#include &lt;vector&gt;
#include &lt;string&gt;
#include &lt;iostream&gt;
#include &lt;fstream&gt;
#include &lt;sstream&gt;
#include &lt;cmath&gt;
#include &lt;boost/algorithm/string.hpp&gt;


Prefer modern initializers for constructors

The constructor use the more modern initializer style rather than the old style you're currently using.  Instead of this:

XYZCoordinate(int X, int Y, float Z){
x = X;
y = Y;
z = Z;
}


I would write this:

XYZCoordinate(int X, int Y, float Z) : x{X}, y{Y}, z{Z} {}


However, in this case, for such a simple class, in real code I wouldn't write a constructor at all and would simply delete the line.  The class can still be initialized like this: XYZCoordinate xyz{3, 5, -23.22}; via aggregate initialization.  

Use better variable names

The variable names xmax, xmin, etc. are good, but the name file is not.  The first names explain something about what the variable means within the context of the code, but the latter is only confusing. A better name might be coordinates.  Similarly, Loadfileoutput is another poor name -- I'd suggest Coordinates for the name of that class.

Use C++ idiom

The code currently contains this:

vector&lt;XYZCoordinate&gt; file = vector&lt;XYZCoordinate&gt;();


That's really not necessary.  Just write this instead:

vector&lt;XYZCoordinate&gt; coordinates{};


Prefer a struct to a class for objects with no invariants

C++ has two primary means to encapsulate some behavior and data: struct and class.  The only difference is that a struct has its members public by default, while the class members are private by default.  If there is no invariant, (a thing that must always be true for the class to be coherent), then it often makes sense to use a struct instead.  In this case the XYZCoordinate class has no invariants and is probably better suited as a struct.

Write a custom extractor

Here's the code I'd prefer to write to read in and create the desired object:

std::ifstream in(filename);
Coordinates c;
in &gt;&gt; c;


This can be done by writing an extractor for the data.  In this case, I'd write two extractors.  One for the Coordinates class and one for the XYZCoordinate class.  Here's one way to write one for the XYZCoordinate class.  Here's the complete definition for that class, which I have converted to a struct:

struct XYZCoordinate{
    int x;
    int y;
    float z;
    friend std::istream&amp; operator&gt;&gt;(std::istream&amp; in, XYZCoordinate &amp;xyz) {
        float a, b;
        in &gt;&gt; a &gt;&gt; b &gt;&gt; xyz.z;
        xyz.x = std::rint(a);
        xyz.y = std::rint(b);
        return in;
    }
};


Now we can write an extractor for the Coordinates class.  Here's that complete class:

class Coordinates
{
    int xmin;
    int xmax;
    int ymin;
    int ymax;
    std::vector&lt;XYZCoordinate&gt; values;

public:
    friend std::ostream&amp; operator&lt;&lt;(std::ostream&amp; out, const Coordinates &amp;c) {
        return out &lt;&lt; "[ " &lt;&lt; c.xmin &lt;&lt; ", " &lt;&lt; c.xmax &lt;&lt; "], [ " 
            &lt;&lt; c.ymin &lt;&lt; ", " &lt;&lt; c.ymax &lt;&lt; "], with " &lt;&lt; c.values.size() &lt;&lt; " points\n";
    }
    friend std::istream&amp; operator&gt;&gt;(std::istream&amp; in, Coordinates &amp;c) {
        // skip comment lines
        while (in.peek() == '#') {
            in.ignore(std::numeric_limits&lt;std::streamsize&gt;::max(), '\n'); 
        }
        XYZCoordinate xyz;
        while (in &gt;&gt; xyz) {
            c.values.push_back(xyz);
            c.xmin = std::min&lt;int&gt;(c.xmin, xyz.x);
            c.xmax = std::max&lt;int&gt;(c.xmax, xyz.x);
            c.ymin = std::min&lt;int&gt;(c.xmin, xyz.y);
            c.ymax = std::max&lt;int&gt;(c.xmax, xyz.y);
        }
        return in;
    }
};


Note that the values of xmin, xmax, etc. are invariants for this class, so we leave it as a class with only private data members.  If you need to access the items in the vector, you can add custom functions to handle that.

Provide a test driver

This is more about getting good reviews than the code itself, but it's often very useful if you provide some simple main that exercises the code.  This helps reviewers (and yourself!) understand how it's to be used in context.  In this case, I wrote this:

#include &lt;vector&gt;
#include &lt;iostream&gt;
#include &lt;fstream&gt;
#include &lt;locale&gt;
#include &lt;limits&gt;
#include &lt;algorithm&gt;

// all of the class code above goes here

int main(int argc, char *argv[]) {
    if (argc != 2) {
        std::cerr &lt;&lt; "Usage: xyz infile.txt\n";
        return 1;
    }
    std::ifstream in(argv[1]);
    in.imbue(std::locale(std::locale(), new csv_reader()));
    Coordinates c;
    in &gt;&gt; c;
    std::cout &lt;&lt; c;
}


The call to imbue uses a locale that treats commas as whitespace.  That code is in this excellent answer.  Now you don't have to explicitly check for commas or spaces and don't need boost.  Everything here is standard C++.

Results

I wrote a program to create a set of 20 millon sets of random coordinates.  Using that input, your original program took 41.9 seconds on my machine, while the one presented above took 14.8 seconds.