Re: [PATCH] Marking weak alist vectors

[Ludovic Courtès]

Han-Wen Nienhuys <address@hidden> writes:

> Does your patch solve the problem that cyclical structures (values that 
> point back to keys) should also be GC-ed?

I guess you're talking about cyclical structures in doubly-weak alist
vectors.  If so, it apparently does since if both WEAK_VALUES and
WEAK_KEYS are false in both functions, then neither the key nor the
value will ever be marked by those functions.

Actually, I've spent some time re-reviewing this patch and I'm having
starting to have a headache.  But anyway, here are a few thoughts.

1.  The tests in `weaks.test' are broken in several ways, not only
    because "we have no way of knowing for certain that the object is
    really dead" as stated there.

    * First, they assume that

        (begin
          (hashq-set! h "a string" 123)
          (hashq-ref  h "a string))

      returns true.  This is wrong since `hashq-ref' uses `eq?' to
      compare keys, and `(eq? "a string" "a string")' returns #f.
      Instead, it could use `hash-map->list'.

    * Second, it should perform a `(read-disable 'positions)' since
      source properties rely on a weakly-keyed hash table where keys are
      source expressions.

2.  The C test I submitted, unlike `weaks.test', can *reliably*
    determine whether an object was swept.  However, it is clearly not
    as complete as `weaks.test'.

3.  Given the level of non-determinism I've been able to observe, I'm
    afraid leaks are causing us difficulties.  For instance, while
    testing weakly-key alist vectors "by hand" in a REPL, it occurred to
    me that the weak-key pair would reliably die, *unless* the hash
    table was written (I mean using `write'):

      guile> (define h (make-doubly-weak-alist-vector 12))
      guile> (hashq-set! h "sdf" "paf")
      guile> (hashq-set! h "hello" "world")
      guile> (gc)
      guile> h
      #w(() () () () () () () () () () () ())

    The same but print H before calling `gc':

      guile> (hashq-set! h "sdf" "paf")
      guile> (hashq-set! h "hello" "world")
      guile> h
      #w((("hello" . "world") ("sdf" . "paf")) () () () () () () () () () () ())
      guile> (gc)
      guile> (gc)
      guile> (gc)
      guile> h
      #w((("hello" . "world") ("sdf" . "paf")) () () () () () () () () () () ())

4.  Looking a Bruno Haible's paper[0] on this topic, it seems that getting
    it right is, well, pretty hard.  ;-)

> Why are you storing SCM references as properties?  It's more efficient 
>   both in time and space to use a SCM member in my_object_t.

That's the whole point of the test: object properties are used because
they involve weak hash tables.

Thanks,
Ludovic.

[0] http://www.haible.de/bruno/papers/cs/weak/WeakDatastructures-writeup.html