apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <jwool...@virginia.edu>
Subject Re: apr_buckets_file.c:file_read + XTHREAD
Date Wed, 28 Nov 2001 04:46:52 GMT
On Tue, 27 Nov 2001, William A. Rowe, Jr. wrote:

> Better yet.
>
> If APR_MMAP_LIMIT is exceeded, break it into n APR_MMAP_LIMIT sized buckets,
> wherein only one bucket may be mapped at a time.
> What I'm suggesting is that as each bucket is consumed, it is un-memaped.
> The buckets are only mmaped as they are read.  This way, we get the benefits
> of mmap, without the pain of page consumption.

Absolutely.

That's actually what I had in mind.  I forgot about the friggin pool
cleanup doing the apr_mmap_delete() for us rather than mmap_destroy()
doing it.  Feh.

Maybe the newly-introduced mmap "ownership" concept can help us here.  If
the mmap bucket holds the apr_mmap_t that "owns" the OS mmap, then the
buckets code's mmap_destroy() can safely munmap the thing.  We should now
actually always be able to call apr_mmap_delete() from mmap_destroy()
safely, because apr_mmap_delete() knows about the owner concept and just
returns success if it's not the owner.  That would be nice, actually,
because we wouldn't have to worry about cases like mmap_destroy()
munmaping an mmap owned by mod_file_cache, for example.


So I guess it would look something like this:

Index: apr_buckets_file.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_buckets_file.c,v
retrieving revision 1.61
diff -u -d -r1.61 apr_buckets_file.c
--- apr_buckets_file.c	2001/11/28 00:38:00	1.61
+++ apr_buckets_file.c	2001/11/28 04:26:13
@@ -84,15 +84,25 @@
     apr_bucket_file *a = e->data;
     apr_mmap_t *mm;

-    if (APR_MMAP_CANDIDATE(filelength) &&
-        (apr_mmap_create(&mm, a->fd, fileoffset, filelength,
-                         APR_MMAP_READ, p) == APR_SUCCESS))
+    if (filelength > APR_MMAP_LIMIT) {
+        if (apr_mmap_create(&mm, a->fd, fileoffset, APR_MMAP_LIMIT,
+                            APR_MMAP_READ, p) != APR_SUCCESS) {
+            return 0;
+        }
+        else {
+            apr_bucket_split(e, APR_MMAP_LIMIT);
+            filelength = APR_MMAP_LIMIT;
+        }
+    }
+    else if ((filelength >= APR_MMAP_THRESHOLD) &&
+             (apr_mmap_create(&mm, a->fd, fileoffset, filelength,
+                              APR_MMAP_READ, p) != APR_SUCCESS))
     {
-        apr_bucket_mmap_make(e, mm, 0, filelength);
-        file_destroy(a);
-        return 1;
+        return 0;
     }
-    return 0;
+    apr_bucket_mmap_make(e, mm, 0, filelength);
+    file_destroy(a);
+    return 1;
 }
 #endif

Index: apr_buckets_mmap.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_buckets_mmap.c,v
retrieving revision 1.44
diff -u -d -r1.44 apr_buckets_mmap.c
--- apr_buckets_mmap.c	2001/11/21 17:00:51	1.44
+++ apr_buckets_mmap.c	2001/11/28 04:26:13
@@ -80,8 +80,7 @@
     apr_bucket_mmap *m = data;

     if (apr_bucket_shared_destroy(m)) {
-        /* no need to apr_mmap_delete(m->mmap) here... it will
-         * get done automatically when the pool gets cleaned up. */
+        apr_mmap_delete(m->mmap);
         free(m);
     }
 }


[Tested to compile but not to work ;]

Thoughts?

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA





Mime
View raw message