Performance issue with parser written with Boost::spirit

Question

I want to parse a file that looks like this (FASTA-like text format):

    >InfoHeader
    "Some text sequence that has a line break after every 80 characters"
    >InfoHeader
    "Some text sequence that has a line break after every 80 characters"
    ...

e.g.:

    >gi|31563518|ref|NP_852610.1| microtubule-associated proteins 1A/1B light chain 3A isoform b [Homo sapiens]
    MKMRFFSSPCGKAAVDPADRCKEVQQIRDQHPSKIPVIIERYKGEKQLPVLDKTKFLVPDHVNMSELVKI
    IRRRLQLNPTQAFFLLVNQHSMVSVSTPIADIYEQEKDEDGFLYMVYASQETFGFIRENE

I wrote a parser for this with boost::spirit. The parser correctly stores the header line and the following text sequence in a std::vector< std::pair< string, string >> but it takes kind of long for bigger files (17sec for a 100MB file). As comparison I wrote a program without boost::spirit (just STL functions) that simply copies each line of that 100MB file in a std::vector. The whole process takes less than a second. The "program" used for the comparison is not serving the purpose but I don't think the parser should take that much longer...

I know there are plenty of other FASTA parsers around but I'm rather curious why my code is slow.

The .hpp file:

#include <boost/filesystem/path.hpp>

namespace fs = boost::filesystem;


class FastaReader {

public:
    typedef std::vector< std::pair<std::string, std::string> > fastaVector;

private:
    fastaVector fV;
    fs::path file;  

public:
    FastaReader(const fs::path & f);
    ~FastaReader();

    const fs::path & getFile() const;
    const fastaVector::const_iterator getBeginIterator() const;
    const fastaVector::const_iterator getEndIterator() const;   

private:
    void parse();

};

And the .cpp file:

#include <iomanip>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <boost/filesystem/fstream.hpp>
#include <boost/filesystem/operations.hpp>
#include <boost/filesystem/path.hpp>
#include <boost/spirit/include/classic_position_iterator.hpp>
#include <boost/spirit/include/phoenix_bind.hpp>
#include <boost/spirit/include/phoenix_core.hpp>
#include <boost/spirit/include/phoenix_fusion.hpp>
#include <boost/spirit/include/phoenix_operator.hpp>
#include <boost/spirit/include/qi.hpp>
#include "fastaReader.hpp"


using namespace std;

namespace fs = boost::filesystem;
namespace qi = boost::spirit::qi;
namespace pt = boost::posix_time;

template <typename Iterator, typename Skipper>
struct FastaGrammar : qi::grammar<Iterator, FastaReader::fastaVector(), qi::locals<string>, Skipper> {
    qi::rule<Iterator> infoLineStart;
    qi::rule<Iterator> inputEnd;
    qi::rule<Iterator> lineEnd;
    qi::rule<Iterator, string(), Skipper> infoLine;
    qi::rule<Iterator, string(), Skipper> seqLine;
    qi::rule<Iterator, FastaReader::fastaVector(), qi::locals<string>, Skipper> fasta;


    FastaGrammar() : FastaGrammar::base_type(fasta, "fasta") {
        using boost::spirit::standard::char_;
        using boost::phoenix::bind;
        using qi::eoi;
        using qi::eol;
        using qi::lexeme;
        using qi::_1;
        using qi::_val;
        using namespace qi::labels;

        infoLineStart = char_('>');
        inputEnd = eoi;

        /* grammar */       
        infoLine = lexeme[*(char_ - eol)];
        seqLine = *(char_ - infoLineStart);

        fasta = *(infoLineStart > infoLine[_a = _1] 
            > seqLine[bind(&FastaGrammar::addValue, _val, _a, _1)]
            )
            > inputEnd
        ;

        infoLineStart.name(">");
        infoLine.name("sequence identifier");
        seqLine.name("sequence");

    }

    static void addValue(FastaReader::fastaVector & fa, const string & info, const string & seq) {
        fa.push_back(make_pair(info, seq));
    }
};


FastaReader::FastaReader(const fs::path & f) {
    this->file = f; 
    this->parse();
}


FastaReader::~FastaReader() {}


const fs::path & FastaReader::getFile() const {
    return this->file;
}


const FastaReader::fastaVector::const_iterator FastaReader::getBeginIterator() const {
    return this->fV.cbegin();
}


const FastaReader::fastaVector::const_iterator FastaReader::getEndIterator() const {
    return this->fV.cend();
}


void FastaReader::parse() {
    if ( this->file.empty() ) throw string("FastaReader: No file specified.");
    if ( ! fs::is_regular_file(this->file) ) throw (string("FastaReader: File not found: ") + this->file.string());

    typedef boost::spirit::istream_iterator iterator_type;
    typedef boost::spirit::classic::position_iterator2<iterator_type> pos_iterator_type;
    typedef FastaGrammar<pos_iterator_type, boost::spirit::ascii::space_type> fastaGr;

    fs::ifstream fin(this->file);
    if ( ! fin.is_open() ) {
        throw (string("FastaReader: Access denied: ") + this->file.string());
    }

    fin.unsetf(ios::skipws);

    iterator_type begin(fin);
    iterator_type end;

    pos_iterator_type pos_begin(begin, end, this->file.string());
    pos_iterator_type pos_end;

    fastaGr fG;
    try {
        std::cerr << "Measuring: Parsing." << std::endl;
        const pt::ptime startMeasurement = pt::microsec_clock::universal_time();

        qi::phrase_parse(pos_begin, pos_end, fG, boost::spirit::ascii::space, this->fV);

        const pt::ptime endMeasurement = pt::microsec_clock::universal_time();
        pt::time_duration duration (endMeasurement - startMeasurement);
        std::cerr << duration <<  std::endl;
    } catch (std::string str) {
        cerr << "error message: " << str << endl;
    }   
}

So the grammar does the folloing: It looks for a ">" sign and then stores all following characters until an EOL is detected. After the EOL the text sequence starts and ends when a ">" sign is detected. Both strings (header line and text sequence) are then stored in a std::vector by calling FastaReader::addValue().

I compiled my program using g++ version 4.8.2 with -O2 and -std=c++11 flags.

So where is the the performance issue in my code?

Answer 1

_{Next: Step 2. Faster with mmap}

Step 1. Cleaning up + Profiling

You should avoid the many rules they introduce type erasure.

If you input is sane, you can do without the skipper (anyways, line ends were significant, so it made no sense to skip them).

Use fusion adaptation instead of a helper to construct new pairs:

This is not optimal, yet, but a lot cleaner:

$ ./test1
Measuring: Parsing.
00:00:22.681605

Slightly more efficient by reducing moving parts and indirections:

Live On Coliru

#include <boost/filesystem/path.hpp>

namespace fs = boost::filesystem;

class FastaReader {    
public:
    typedef std::pair<std::string, std::string> Entry;
    typedef std::vector<Entry> Data;

private:
    Data fV;
    fs::path file;  

public:
    FastaReader(const fs::path & f);
    ~FastaReader();

    const fs::path & getFile() const;
    const Data::const_iterator begin() const;
    const Data::const_iterator end() const;   

private:
    void parse();    
};

#include <iomanip>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <boost/filesystem/fstream.hpp>
#include <boost/filesystem/operations.hpp>
#include <boost/filesystem/path.hpp>

#include <boost/spirit/include/classic_position_iterator.hpp>
#include <boost/spirit/include/qi.hpp>
#include <boost/fusion/adapted/std_pair.hpp>
//#include "fastaReader.hpp"

using namespace std;

namespace fs = boost::filesystem;
namespace qi = boost::spirit::qi;
namespace pt = boost::posix_time;

template <typename Iterator>
struct FastaGrammar : qi::grammar<Iterator, FastaReader::Data()> {
    qi::rule<Iterator, FastaReader::Data()> fasta;

    FastaGrammar() : FastaGrammar::base_type(fasta) {
        using namespace qi;

        fasta = *('>' >> *~char_('\n') >> '\n' 
                      >> *~char_('>')) 
                >> *eol
                >> eoi
                ;

        BOOST_SPIRIT_DEBUG_NODES((fasta));
    }
};


FastaReader::FastaReader(const fs::path & f) : file(f) {
    parse();
}

FastaReader::~FastaReader() {}

const fs::path & FastaReader::getFile() const {
    return this->file;
}

const FastaReader::Data::const_iterator FastaReader::begin() const {
    return this->fV.cbegin();
}

const FastaReader::Data::const_iterator FastaReader::end() const {
    return this->fV.cend();
}

void FastaReader::parse() {
    if (this->file.empty())                throw std::runtime_error("FastaReader: No file specified.");
    if (! fs::is_regular_file(this->file)) throw std::runtime_error(string("FastaReader: File not found: ") + this->file.string());

    typedef boost::spirit::istream_iterator                           iterator_type;
    typedef boost::spirit::classic::position_iterator2<iterator_type> pos_iterator_type;
    typedef FastaGrammar<pos_iterator_type>                           fastaGr;

    fs::ifstream fin(this->file);
    if (!fin) {
        throw std::runtime_error(string("FastaReader: Access denied: ") + this->file.string());
    }

    static const fastaGr fG{};
    try {
        std::cerr << "Measuring: Parsing." << std::endl;
        const pt::ptime startMeasurement = pt::microsec_clock::universal_time();

        pos_iterator_type first(iterator_type{fin >> std::noskipws}, {}, file.string());
        qi::phrase_parse<pos_iterator_type>(first, {}, fG, boost::spirit::ascii::space, this->fV);

        const pt::ptime endMeasurement = pt::microsec_clock::universal_time();
        pt::time_duration duration (endMeasurement - startMeasurement);
        std::cerr << duration <<  std::endl;
    } catch (std::exception const& e) {
        cerr << "error message: " << e.what() << endl;
    }   
}

int main() {
    std::ios::sync_with_stdio(false);

    FastaReader reader("input.txt");

    //for (auto& e : reader) std::cout << '>' << e.first << '\n' << e.second << "\n\n";
}

This is still slow. Let's see what takes so long:

That's pretty, but hardly tells us what we need to know. This however does: top-N time consumers are

So most time is spent in istream iteration and the multi-pass adaptor. You could argue that the multipass adaptor could be optimized for by flushing it once in a while (each line?) but really, we would prefer not to be tied to the whole stream and operator on the (stream) buffer instead.

So, I though let's use a mapped file instead:

Next: Step 2. Faster with mmap