Re: mmap for guile

[Maxime Devos]

Some old mmap things that might be useful:

* https://lists.nongnu.org/archive/html/guile-devel/2013-04/msg00235.html
* https://lists.gnu.org/archive/html/bug-guile/2017-11/msg00033.html
* https://lists.gnu.org/archive/html/guile-user/2006-11/msg00013.html

+SCM_DEFINE (scm_mmap_search, "mmap/search", 2, 4, 0, 
+            (SCM addr, SCM len, SCM prot, SCM flags, SCM fd, SCM
offset),
+           "See the unix man page for mmap.  Returns a bytevector.\n"
+           "Note that the region allocated will be searched by the
garbage\n"
+           "collector for pointers.  Defaults:\n"

I think it would be a good idea to document it will be automatically
unmapped during GC, as this is a rather low-leel interface

Also, what if you mmap a region, use bytevector->pointer and pass it to
some C thing, which saves the pointer somewhere where boehm-gc can find
it and boehm-gc considers it to be live, is there something that
prevents boehm-gc from improperly calling the finalizer & unmapping the
region, causing a dangling pointer?

Also, WDYT of using ports instead of raw fds in the API?  That would
play nicer with move->fdes etc.

> +  GC_exclude_static_roots(ptr, (char*)ptr + len);

After an unmap, will the GC properly forget the roots information?

>+  /* Invalidate further work on this bytevector. */
>+  SCM_BYTEVECTOR_SET_LENGTH (bvec, 0);
>+  SCM_BYTEVECTOR_SET_CONTENTS (bvec, NULL);

Possibly Guile's optimiser assumes that bytevectors never change in
length (needs to be checked).  So unless the relevant optimiser code is
changed, and it is documented that bytevectors can change in length, I
think it would be safer to not have an unmapping procedure in Scheme
(though a procedure for remapping it as /dev/zero should be safe).

> +  bvec = scm_c_take_typed_bytevector((signed char *) c_mem + c_offset, c_len,
> +                                  SCM_ARRAY_ELEMENT_TYPE_VU8, pointer);

Would scm_pointer_to_bytevector fit here?  Also, scm_c_make_typed_bytevector
looks like something that can cause an out-of-memory-exception but the
finaliser hasn't been set yet.


>+// call fstat to get file size
>+SCM_DEFINE (scm_mmap_file, "mmap-file", 1, 1, 0, 
>+            (SCM file, SCM prot),
>+          "This procedure accepts a file in the form of filename,\n"
>+            " file-port or fd.  It returns a bytevector.  It must
>not\n"
>+            " contain scheme allocated objects as it will not be\n"
>+            " searched for pointers. Default @var{prot} is @code{\"r\"}.")

I would restrict the C code to only ports and file descriptors, and leave file
names to a Scheme wrapper.  That way, you automatically get appropriate E...
errors in case open-file fails (and maybe &i/o-filename etc. if the core Guile
FS functions are later changed to R6RS), less chance on accidentally forgetting
to close a fd (*) ...

(*) One possible problem: if the file is opened, and mmap fails, then you still
need to close the file port (and rethrow), so some exception handling is still
required, though no C-style exception handling ...

> +/* The following copied from bytevectors.c. Kludge? */
> +#define SCM_BYTEVECTOR_SET_LENGTH(_bv, _len)            \
> +  SCM_SET_CELL_WORD_1 ((_bv), (scm_t_bits) (_len))
> +#define SCM_BYTEVECTOR_SET_CONTENTS(_bv, _contents)  \
> +  SCM_SET_CELL_WORD_2 ((_bv), (scm_t_bits) (_contents))

To avoid accidental problems if bytevectors.c is modified later, I'd add add a 
comment
in bytevectors.c referring to this file, to add a reminder that mmunmap
makes assumptions about the layout.

> +  scm_c_define ("PAGE_SIZE", scm_from_int (getpagesize()));

From local man page:

       SVr4,  4.4BSD,  SUSv2.   In SUSv2 the getpagesize() call is labeled 
LEGACY, and in
       POSIX.1-2001 it has been dropped; HP-UX does not have this call.

NOTES
       Portable applications should  employ  sysconf(_SC_PAGESIZE)  instead  of 
 getpage‐
       size():

Greetings,
Maxime.

signature.asc
Description: This is a digitally signed message part