[OPEN-ILS-DEV] Dead functions

[Scott McKellar]

--- On Fri, 12/19/08, Mike Rylander <mrylander at gmail.com> wrote:
<snip>

> > When you say YES * 1000, I'm not sure if you mean
> yes let's get rid of
> > these functions or yes let's use mmap().  In case
> 
> I was jumping for joy over removing the dead code, but then
> I got to
> thinking about dump/restore.  For dump/restore (or just
> FS-backed
> persistence of some pile) the "restore" step
> really needs to be record
> oriented.  We don't want to blow up RAM because we have
> a lot of stuff
> to work on.  Sooooo... I think we should just set that part
> aside for
> now and remove the dead code.

On any system where such a dump/restore utility is likely to run, we
should be able to load several megabytes into RAM without a problem.
Hundreds of megabytes could be a problem.  Gigabytes could be a problem.
I don't know what size of files you have in mind, but we should probably
design for very large files.

A related issue is that the JSON parser expects to see the entire
jSON string at once, and to produce one jsonObject to represent all of
it.  That means that we would need not only enough memory to hold the
JSON string, but also enough memory to hold the resulting big 
jsonObject.

To process a huge file without requiring huge memory we need a parser
that works incrementally -- line-oriented, as you put it.  By analogy
with XML, we'd need SAX, not DOM.  That means that a dump/restore
utility capable of handling large files would involve a lot more than
just gluing together a few existing components.

=============

After my previous post I realized that the way file_to_string() works is
just silly anyway.  (I'm entitled to say that because I wrote the
current implementation myself.)  There is no reason to use a
growing_buffer at all.  Just do an fstat() to get the size of the file,
allocate a buffer accordingly, and load the whole thing with a single
read().  Then you get O(n) performance instead of O(n-squared).

==============

Also I did some further experimenting with mmap(), using a different
benchmark program that just loads a file without trying to parse it as
JSON.  In one trial I malloc a buffer and read a file into it (8.7 MB
worth).  In the other trial I call mmap() and munmap() for the same
amount of space.

The result is that mmap() and munmap() runs in about 22 microsec, while
the malloc/read/free version takes about 700-900 times as long.
Apparently mmap() just assigns memory locations to file offsets but
doesn't do any IO until you try to access that memory.

In another variant I included a small loop to examine every byte in the
buffer, thus forcing the system to actually read the file into the
mmapped memory.  In this case the mmapped version was still faster.
Obviously it was no longer hundreds of times faster because the savings
were diluted by the overhead of the additional loop, but it was still
faster.

However the savings in absolute time were somewhat reduced, from 
about 170 millisec to about 140 millisec (the timings are somewhat
variable, so it's hard to be very precise here).  The reduction in
savings reflects the overhead of the necessary IO.  The remaining
savings reflects the fact that we bypass the kernel IO buffers and the
associated layer of copying.

================

After looking further into mmap(), I'm not sure that we would really
want to use it, despite the potential savings.

One reason is that I'm not sure if we can guarantee that the mmapped
data will be nul-terminated.  Most of the time the last byte will be
in the middle of a memory page, and mmap() will fill the rest of the
page with binary zeros.  If the file size is a multiple of the page
size, mmap() doesn't add any such zero bytes.  There may or may not be
a way to add a terminal nul ourselves.  I haven't tried.  We might 
need to modify the parser so that it doesn't need to see a terminal nul.

Another reason is that I'm not sure what happens if we fail to unmap
the mapped memory before terminating.  I haven't found a firm statement
either way, but I have found hints that the memory may remain mapped
even after the process terminates.  We don't want to clutter up the
memory space with long unusable stretches just because the dump/restore
utility got killed by a signal.

Unless we can resolve these issues to our satisfaction, I think we 
should expect to use conventional file reads instead of mmap().

==================

in the meanwhile I vote for eliminating the dead functions.  None of
them will be difficult to re-create if we change our minds later.

Scott McKellar
http://home.swbell.net/mck9/ct/