ruricolist / serapeum

Utilities beyond Alexandria
MIT License
420 stars 41 forks source link

Maybe add STATIC-LET #107

Closed phoe closed 2 years ago

phoe commented 2 years ago

There's an article at http://jacek.zlydach.pl/blog/2020-01-11-static-variables-in-common-lisp.html that describes a utility called static-let which allocates a chunk of memory only once and then allows a body of code to refer to it, not unlike a local static variable in C.

Do you think it's worth polishing and adding to Serapeum?

ruricolist commented 2 years ago

Interesting. It seems like a potentially useful idea and not one that's trivial to get right. I think one could probably hook into UIOP to clear the cells before dumping an executable, and initialization at least could be made thread-safe. (And static-let*, would that be useful?)

I'll give it some thought.

phoe commented 2 years ago

A let* variant would be useful and simple to implement in terms of a nested let.

Initialization can be made thread-safe using some sort of condition variable or semaphore, and the initial (list nil) can be modified to instead be (cons value initializedp).

UIOP is not a dependency for Serapeum at the moment - should it become one?

phoe commented 2 years ago

I think I have an idea for an implementation that:

Does that sound OK? If yes, I'll start hacking away at it.

ruricolist commented 2 years ago

Sounds great!

I don't see value in explicitly depending on UIOP, any more than ASDF.

phoe commented 2 years ago

Does it mean that I should use UIOP symbols without an explicit :depends-on? I can do that.

phoe commented 2 years ago

It seems that the only kind of thread-safe guarantee that this code can give is that no two threads will attempt to initialize the static binding at the same time. Everything else, including actually accessing the binding or cleaning the buffers from the outside, will require the user to do their own synchronization, because the body to be executed is opaque to us and the only thing that we can do with it is to slap a big ole with-lock around it, which is almost always generally wrong.

How does this sound?

ruricolist commented 2 years ago

That sounds right.

I'm particularly thinking that in cases where the static binding is a data structure with its own synchronization, it's asking for bugs for the programmer to have to also remember to think about protecting the storage cell itself.

phoe commented 2 years ago

I am thinking of something like that:

(defstruct (static-binding (:constructor %make-static-binding)
                           (:copier nil)
                           (:predicate nil)
                           (:conc-name #:sb-))
  (value nil :type t)
  (initializedp nil :type boolean)
  (lock nil :read-only t :type (or null bt:lock)))

(defun make-static-binding (&key synchronizedp)
  (%make-static-binding
   :lock (if synchronizedp (bt:make-lock "Static binding lock") nil)))

As for actual initialization (the double unless initializedp is a feature):

(defun make-initform (x)
  (with-canonicalized-binding-accessors ()
    (let* ((sym (sym x))
           (body `(setf (value ,sym) ,(value x)
                        (initializedp ,sym) t)))
      `(unless (initializedp ,sym)
         ,(if (synchronize-initform-p x)
              `(bt:with-lock-held ((lock ,sym))
                 (unless (initializedp ,sym)
                   ,body))
              body)))))

I piggyback on the fact that structure writes are usually atomic, so if anyone wants to write to sb-value, that should work well enough.

Is that what you mean by "protecting the storage cell itself"? We can't really do anything more because the moment we return the object stored in sb-value, the responsibility to properly synchronize it is no longer ours.

ruricolist commented 2 years ago

Yes, double-checked locking during initialization is exactly what I had in mind.

phoe commented 2 years ago

OK! Thanks.

Attaching an example implementation that I hacked together in an hour or three, very roughly tested in the REPL.

I've modified the API compared to the one introduced by @temporal in order for the final form of each binding to be effectively (var val &type synchronize-initform-p) rather than having a single type argument in the middle.

Seems to pass the smoke test:

SERAPEUM/STATIC-LET> (defun foo () 
                       (static-let ((x (progn (print "haha") (make-array 100))))
                         x))
FOO

SERAPEUM/STATIC-LET> (foo)
"haha" 
#(0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
  0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
  0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0)

SERAPEUM/STATIC-LET> (foo)
#(0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
  0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
  0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0)

Code review welcome - I'll make a PR if it's good enough.

static-let.txt

phoe commented 2 years ago

PR made, let me see if the CI succeeds. Once it does, we can think of writing some meaningful tests for the synchronized version.