Dont use mmap() for anything, axe the code instead

- Commit 4cb02aa928 asked to see
  what breaks when mmap() is used, now we know: large package support
  broke when enabling it. Could be fixed of course by eg adding
  a size cap to the fsm part as well, but just doesn't seem worth it:
  I fail to measure any meaningful performance improvement from mmap
  usage in either case, and added complexity for what is close to
  zero benefit just doesn't make sense... and various sources in fact
  note the rpm usage (read through the entire file sequentially) as one
  of the cases where mmap() is NOT beneficial due to mmap() high
  setup + teardown cost + page fault speed (or lack of thereof).
This commit is contained in:
Panu Matilainen 2012-07-02 14:30:24 +03:00
parent 105f91fc9a
commit bf3a14a866
3 changed files with 0 additions and 79 deletions

View File

@ -480,8 +480,6 @@ AC_TYPE_PID_T
AC_TYPE_SIZE_T AC_TYPE_SIZE_T
dnl Checks for library functions. dnl Checks for library functions.
AC_FUNC_MMAP
AC_CHECK_FUNCS(putenv) AC_CHECK_FUNCS(putenv)
AC_CHECK_FUNCS(mempcpy) AC_CHECK_FUNCS(mempcpy)
AC_CHECK_FUNCS(fdatasync) AC_CHECK_FUNCS(fdatasync)

View File

@ -7,9 +7,6 @@
#include <utime.h> #include <utime.h>
#include <errno.h> #include <errno.h>
#if defined(HAVE_MMAP)
#include <sys/mman.h>
#endif
#if WITH_CAP #if WITH_CAP
#include <sys/capability.h> #include <sys/capability.h>
#endif #endif
@ -864,12 +861,6 @@ static int writeFile(FSM_t fsm, int writeData, rpmcpio_t archive)
if (writeData && S_ISREG(st->st_mode)) { if (writeData && S_ISREG(st->st_mode)) {
size_t len; size_t len;
#ifdef HAVE_MMAP
char * buf = NULL;
void * mapped = MAP_FAILED;
size_t nmapped;
int xx;
#endif
rfd = Fopen(fsm->path, "r.ufdio"); rfd = Fopen(fsm->path, "r.ufdio");
if (Ferror(rfd)) { if (Ferror(rfd)) {
@ -877,28 +868,9 @@ static int writeFile(FSM_t fsm, int writeData, rpmcpio_t archive)
goto exit; goto exit;
} }
/* XXX unbuffered mmap generates *lots* of fdio debugging */
#ifdef HAVE_MMAP
nmapped = 0;
mapped = mmap(NULL, st->st_size, PROT_READ, MAP_SHARED, Fileno(rfd), 0);
if (mapped != MAP_FAILED) {
buf = fsm->buf;
fsm->buf = (char *) mapped;
len = nmapped = st->st_size;
#if defined(MADV_DONTNEED)
xx = madvise(mapped, nmapped, MADV_DONTNEED);
#endif
}
#endif
left = st->st_size; left = st->st_size;
while (left) { while (left) {
#ifdef HAVE_MMAP
if (mapped != MAP_FAILED) {
len = nmapped;
} else
#endif
{ {
len = (left > fsm->bufsize ? fsm->bufsize : left); len = (left > fsm->bufsize ? fsm->bufsize : left);
if (Fread(fsm->buf, sizeof(*fsm->buf), len, rfd) != len || Ferror(rfd)) { if (Fread(fsm->buf, sizeof(*fsm->buf), len, rfd) != len || Ferror(rfd)) {
@ -914,17 +886,6 @@ static int writeFile(FSM_t fsm, int writeData, rpmcpio_t archive)
left -= len; left -= len;
} }
#ifdef HAVE_MMAP
if (mapped != MAP_FAILED) {
xx = msync(mapped, nmapped, MS_ASYNC);
#if defined(MADV_DONTNEED)
xx = madvise(mapped, nmapped, MADV_DONTNEED);
#endif
xx = munmap(mapped, nmapped);
fsm->buf = buf;
}
#endif
} else if (writeData && S_ISLNK(st->st_mode)) { } else if (writeData && S_ISLNK(st->st_mode)) {
size_t len = strlen(symbuf); size_t len = strlen(symbuf);
if (rpmcpioWrite(archive, symbuf, len) != len) { if (rpmcpioWrite(archive, symbuf, len) != len) {

View File

@ -13,10 +13,6 @@
#endif #endif
#if defined(HAVE_MMAP)
#include <sys/mman.h>
#endif
#include <sys/types.h> #include <sys/types.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <sys/wait.h> #include <sys/wait.h>
@ -154,43 +150,9 @@ int rpmDoDigest(int algo, const char * fn,int asAscii,
goto exit; goto exit;
} }
/* file to large (32 MB), do not mmap file */
if (fsize > (size_t) 32*1024*1024)
if (ut == URL_IS_PATH || ut == URL_IS_UNKNOWN)
ut = URL_IS_DASH; /* force fd io */
switch(ut) { switch(ut) {
case URL_IS_PATH: case URL_IS_PATH:
case URL_IS_UNKNOWN: case URL_IS_UNKNOWN:
#ifdef HAVE_MMAP
if (pid == 0) {
int xx;
DIGEST_CTX ctx;
void * mapped;
if (fsize) {
mapped = mmap(NULL, fsize, PROT_READ, MAP_SHARED, fdno, 0);
if (mapped == MAP_FAILED) {
xx = close(fdno);
rc = 1;
break;
}
#ifdef MADV_SEQUENTIAL
xx = madvise(mapped, fsize, MADV_SEQUENTIAL);
#endif
}
ctx = rpmDigestInit(algo, RPMDIGEST_NONE);
if (fsize)
xx = rpmDigestUpdate(ctx, mapped, fsize);
xx = rpmDigestFinal(ctx, (void **)&dig, &diglen, asAscii);
if (fsize)
xx = munmap(mapped, fsize);
xx = close(fdno);
break;
}
#endif
case URL_IS_HTTPS: case URL_IS_HTTPS:
case URL_IS_HTTP: case URL_IS_HTTP:
case URL_IS_FTP: case URL_IS_FTP: