tags:

views:

321

answers:

5

Hi,

I'm trying to project a file into memory to operate with it. The file contais structs so I'm trying to use a pointer to the start of one struct and then read it and modify some variable. The problem is that the time of execution is high and I suppose that using mmap the time will be less. This is the code, any suggestion?

#include <unistd.h>
#include <sys/stat.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>

int revisanotas(int fd)
{
int nbytes=1;
int nbytese=0;
int i=0;
int n=0;
struct stat datos;
fstat(fd, &datos);
evaluacion buf;
evaluacion* buffer=mmap(0,datos.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
int actual = read(fd,buffer,datos.st_size);
{
i++;
if (buffer[i].notamedia >= 4.5 && buffer[i].notamedia < 5)
{
n=n+1;
printf("Notamedia = %f\n",buffer[i].notamedia);

buffer[i].notamedia=5;
}

}while (i<(datos.st_size/(sizeof(evaluacion))));
return
A: 

Well, first, please tell us what evaluacion is defined as, and put the do in there that matches the while; I'm assuming it's right after the "int actual" line.

Second, it looks like you might be calling mmap() more often than needed; how often is revisanotas() called with the same fd? The mmap call itself is slow, like malloc; the speed is when you use the mapped file, in this case, the data pointed to by buffer.

Third, compute datos.st_size/(sizeof(evaluacion)) once outside the loop and change the while clause to compare to that. The current code looks like it performs the divide once per iteration through the loop, and divides are slow.

See if that helps any.

Mike D.
+2  A: 

The call to read() is unnecessary. Mmap() maps the file contents into memory for you - that is why it is generally faster than reading the entire file using read(). You should be able to remove the call to read() altogether. There are some other problems with your code though.

If you want to modifications to actually be reflected in the disk file, then you should call msync(buffer, dataos.st_size, MS_SYNC). When you are done, call munmap(buffer, dataos.st_size) to release the shared memory segment. Think of msync() as the shared memory equivalent to fflush() and munmap() is similar to close(). The key difference between munmap() and close() is that the former does not flush buffers or synchronize to disk so you have to do it yourself.

D.Shawley
Thank you very much, I removed the read() and the time decreases dramatically :). Now, I'm trying to figure out how to use msync, because what I have to do is modify the notamedia variable in each structure that matches the condition and then write in the disk the whole file modified.
Peter
D.Shawley
A: 

Thanks mike for the answer, the struct is something like this and it's defined in a header file:

struct evaluacion 
{ char id[16]; 
 char apellido1[32]; 
char apellido2[32]; 
char name[32]; float nota1p; 
float nota2p; 
float notamedia; 
char photofilename[20]; 
int photosize; 
char photodata[16000]; 
};

There's only one fd, only one file open, but many structs inside

Peter
A: 

I'm making some progress I think, but seems I'm not modifying the file. If I run the program once, it detects 32 variables modified, but If I run twice still the same, when the It's supposed to be modified :(

The execution time now is not bad I think

int revisanotas(int fd)
{
int i=0;
int n=0;
struct stat datos;
fstat(fd, &datos);
int num=datos.st_size/(sizeof(evaluacion));
evaluacion buf;
evaluacion* buffer=mmap(0,datos.st_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
do
{
i++;
if (buffer[i].notamedia >= 4.5 && buffer[i].notamedia < 5)
{
n=n+1;

buffer[i].notamedia=5;
}
msync(&buffer[i],sizeof(buffer[i]),MS_SYNC);
}while (i<(num));
munmap(buffer,datos.st_size);
return(n);
}
Peter
Check the return value of `msync`. My guess is that it is failing, returning -1, and setting `errno` to `EINVAL` (22). Try calling `msync(buffer, dataos.st_size, MS_SYNC)` before calling `munmap`.
D.Shawley
I did: int r=msync(buffer,datos,st_size,MS_SYNC); mmunmap(buffer,datos.st_size), Seems that msync is returning 0 Correctme if I'm wrong, but do I have to do something to overwrite the original file described by the 'fd'? or once I release the memory and write with msync will be written in the file?
Peter
A: 

Hi,

I think now is working :), the code is as follows. Please comment if you see something wrong:

int revisanotas(int fd)
{
int i=0;
int n=0;
struct stat datos;
fstat(fd, &datos);
int num=datos.st_size/(sizeof(evaluacion));

evaluacion* buffer=mmap(0,datos.st_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
do
{
i++;
if (buffer[i].notamedia >= 4.5 && buffer[i].notamedia < 5)
{
n=n+1;
buffer[i].notamedia=5;
}

}while (i<(num));
int r=munmap(&buffer[0],datos.st_size);

return(n);
}

Thanks everybody for the help.

Peter