C++ slow read/seekg

Question

In my program I read in a file (here only a test file of about 200k data points afterwards there will be millions.) Now what I do is:

for (int i=0;i<n;i++) {
    fid.seekg(4,ios_base::cur);
    fid.read((char*) &x[i],8);
    fid.seekg(8,ios_base::cur);
    fid.read((char*) &y[i],8);
    fid.seekg(8,ios_base::cur);
    fid.read((char*) &z[i],8);
    fid.read((char*) &d[i],8);
    d[i] = (d[i] - p)/p;
    z[i] *= cc;
}

Whereby n denotes the number of points to read in.

Afterwards I write them again with

for(int i=0;i<n;i++){
        fid.write((char*) &d[i],8);
        fid.write((char*) &z[i],8);

        temp = (d[i] + 1) * p;
        fid.write((char*) &temp,8);
    }

Whereby the writing is faster then the reading.(time measured with clock_t)

My Question is now. Have I done some rather stupid mistake with the reading or can this behavior be expected?

I'm using Win XP with a magnetic drive.

yours magu_

Using "seek" in the middle of a read is a bad idea. It would make more sense to dummy-read a value. That's because seek is likely to flush the caches used internally in the `fstream`, so the next read will have to perform a low-level read, rather than fetching data that has been fetched already. — Mats Petersson, May 28 '13 at 13:33
Writing we would expect to be faster than reading. When reading your process has to wait for data. When writing, we can take advantage of the disks data cache. Your process is done working before the disk is done writing your data. It is likely that there are other issues with this as well... your use of seekg looks suspicious. — MobA11y, May 28 '13 at 13:34
The format you write and the format you read doesn't match. I hope it does in the actual code. — Some programmer dude, May 28 '13 at 13:35

huysentruitw · Accepted Answer · 2013-05-28T13:38:58.017

13

You're using seekg too often. I see that you're using it to skip bytes, but you could as well read the complete buffer and then skip the bytes in the buffer:

char buffer[52];

for (int i=0;i<n;i++) {
    fid.read(buffer, sizeof(buffer));
    memcpy(&x[i], &buffer[4], sizeof(x[i]));
    memcpy(&y[i], &buffer[20], sizeof(y[i]));
    // etc
}

However, you can define a struct that represents the data in your file:

#pragma pack(push, 1)
struct Item
{
    char dummy1[4]; // skip 4 bytes
    __int64 x;
    char dummy2[8]; // skip 8 bytes
    __int64 y;
    char dummy3[8]; // skip 8 bytes
    __int64 z;
    __int64 d;
};
#pragma pack(pop)

then declare an array of those structs and read all data at once:

Item* items = new Item[n];
fid.read(items, n * sizeof(Item)); // read all data at once will be amazing fast

(remark: I don't know the types of x, y, z and d, so I assume __int64 here)

edited May 28 '13 at 13:38

answered May 28 '13 at 13:33

huysentruitw

27,376
9
90
133

I always thought that #pragma pack(push, 1) was visual studio only so I was trying to look up the GCC equivalent, but it seems to work in both compilers: http://gcc.gnu.org/onlinedocs/gcc/Structure_002dPacking-Pragmas.html – Sqeaky May 28 '13 at 14:20
Thx very much. This really helped me. My Boss doesn't allow the use of #pragma since the code should be portable to linux. (Even though it should work on gcc...). The buffer gives me speedup of about factor 2 – magu_ May 29 '13 at 06:57
Please use a struct that defines the data structure in your file, it will help to keep the code clean and safe against mistakes. If your Boss ever uses a compiler that doesn't support #pragma pack, you could always use #ifdef's for that compiler and use other packing methods. Example of other compiler: http://stackoverflow.com/questions/10371296/pragma-pack1-nor-attribute-aligned-1-works – huysentruitw May 29 '13 at 08:16

Mats Petersson · Answer 2 · 2013-05-28T13:53:01.250

I personally would (at least) do this:

for (int i=0;i<n;i++) {
    char dummy[8];
    fid.read(dummy,4);
    fid.read((char*) &x[i],8);
    fid.read(dummy,8);
    fid.read((char*) &y[i],8);
    fid.read(dummy,8);
    fid.read((char*) &z[i],8);
    fid.read((char*) &d[i],8);
    d[i] = (d[i] - p)/p;
    z[i] *= cc;
}

Doing a struct, or reading large amounts of data in one go (say adding a second layer, where you read 4KB at a time, and then using a pair of functions that do "skip" and "fetch" of the different fields would be a bit more work, but likely much faster).

Another option is to use mmap in Linux or MapViewOfFile in Windows. This method reduces the overhead in reading a file by a small portion, since there is one less copy required to transfer the data to the application.

Edit: I should add "Make sure you make comparative measurements", and if your application is meant to run on many machines, make sure you make measurements on more than one type of machine, with different alternatives of disk drive, processor and memory. You don't really want to tweak the code so that it runs 50% faster on your machine, but 25% slower on another machine.

Aaron S. Kurland · Answer 3 · 2013-05-28T14:58:46.090

The assert() statements are the most important part of this code so that if your platform ever changes and the width of your native types change then the assertions will fail. Instead of seeking, I would read to a dummy area. The p* variables make the code easier to read, IMO.

assert(sizeof x[0] == 8);
assert(sizeof y[0] == 8);
assert(sizeof z[0] == 8);
assert(sizeof d[0] == 8);

for (int i=0;i<n;i++) {
    char unused[8];
    char * px = (char *) &x[i];
    char * py = (char *) &y[i];
    char * pz = (char *) &z[i];
    char * pd = (char *) &d[i];

    fid.read(unused, 4);
    fid.read(px, 8);
    fid.read(unused, 8);
    fid.read(py, 8);
    fid.read(unused, 8);
    fid.read(pz, 8);
    fid.read(pd, 8);

    d[i] = (d[i] - p)/p;
    z[i] *= cc;
}

I is not in scope where you use it. You state asserts are important, then let the reader divine your meaning instead of providing a rationale. You also made other significant change to code without providing rationale. — Sqeaky, May 28 '13 at 14:23

C++ slow read/seekg

3 Answers3