Re: SBCL Fibers proposal

Hi Anthony,
My notes aren't as detailed as Christophe's review, but I did spend enough time looking at your proposal that I think I should share them anyway. tl;dr is I think dynamic binding needs to be redesigned/rewritten and please start adding citations to the claims to make this easier to review and allow us to have more time to help you fix the sbcl parts you're not sure about rather than spending time to refute the llm on things it says verbosely that are totally wrong. org file is pasted inline in this email.

* Notes on https://atgreen.github.io/repl-yell/posts/sbcl-fibers/ (as of Mar 11)

** 2.5 Fiber join
API seems more or less fine. Why not just copy the
=sb-thread:join-thread= api for values? Someone already thought about
the exception handling for this situation. Is there a reason it should
be substantially different for fibers?

** 2.9 Fiber pinning
=with-fiber-pinned= lambda list looks a bit weird. The optional
argument here makes no sense to me. It should be
=(with-current-fiber-pinned (&key options) &body body)= with just a
body to execute a region with the current fiber pinned (maybe with
options to control =*pinned-blocking-actions*=, and
=with-pinned-fiber= to pin a target fiber. But I also don't really
understand the use-case for pinning a fiber that isn't the currently
running one. What situation is this more flexible than the fiber
itself just knowing when it needs to be pinned? It sounds strictly
safer to only expose something like =(pin-current-fiber)= and
=(unpin-current-fiber)= instead without losing much.

Also, I don't understand the coupling between pinning and having the
fiber be yieldable. Isn't it possible to have the scheduler itself
just not migrate the fiber by checking the pin count? What kind of
safety guaranteed is gained by not letting the fiber yield? If there
is some invariant that could be broken by some fiber being able to
yield, this might be where having some API along the lines of
=fiber-yield-disable= or =fiber-yield-enable= is useful orthogonal to
keeping a fiber pinned to a carrier thread.

** 2.7 Multi-carrier scheduling
It's unclear to me how error handling would work if fibers are grouped
together. For =join-fiber= there's a precedent in =join-thread=, but
the interface around results and exception handling would seem awkward
here. I think a redesign of the interface around scheduling groups
would be good. Maybe we distinguish errors from values by having the
error from a given fiber be an atom in the list and values from a
given fiber as a list. NIL represents no values.

** 2.10
Looks OK, except =fiber-result= is fishy. =fiber-result= could just
return the error object itself and not as a list, which would be a
good way to distinguish the error case from the non-error case. What
should it do anyway when the fiber is still running and hasn't
returned any values yet? But actually, should this even be exposed to
the user? Note that =sb-thread::thread-result= is not exposed and is
an internal implementation detail; the LLM may not understand the
difference here between exposed and unexposed API when it used it for
inspiration. Note that the LLM also hallucinated the =sb-thread= API
in section 2.1; =make-thread= does not take =:initial-bindings= as a
result.

** 4.1 Register Save/Restore Convention
This section looks... wrong, at least without additional context of
where this is happening. Especially for risc-v. How can this possibly
be correct for the Lisp calling convention, which is totally different
when it comes to register save and restore? What about NIL, and the vm
registers?

** 4.2
First, a minor thing: assembler routines stored in static space, not necessarily
alongside other compiled Lisp functions (unless #+immobile-space).

Instead of RET, it seems like this is more like an indirect function
call for return address prediction purposes. It's probably more like
UNWIND in this sense.

** 4.5
Other parts of the backend use SAPs without consing. SAPs are unboxed
in these sorts of situations. I think it might be worth looking into
e.g. the call vops to learn how these types of vops are usually
written. What does the =%fiber-switch= VOP do anyway? It seems out of
place in the design doc in that it has no intro or little relation to
anything else.

** 6.1, 6.2., 6.3, 6.5, 6.6 (dynamic binding stuff)
This design doesn't seem great. Why not just have the equivalent of
tls dynamic binding storage on the fiber itself, instead of all this
overlay stuff? I haven't put too much thought into this but this
doesn't seem like the right thing. Surely there is a design that
avoids all this extra bookkeeping. Maybe you could look more closely
of how TLS works and try to mirror the design onto fibers more
directly; to me this seems like adding layers of hacks to substitute
for something missing in the data structure.


** 6.4 catch/unwind block
This section seems more reasonable in comparison.

** Section 7

This section is a bit of a mess. Could you clean it up a bit?
e.g. references to previous implementations the LLM tried, etc. Also,
the correctness argument seems hallucinated. It's not really
reviewable for me as is.

** Section 8, 9 10, and 11
A lot of the stuff in these sections seems like it could be standard
in the literature and not sbcl specific. Can you cite literature
references in the sections that aren't sbcl-specific here for what the
designs are based on?
*** 8.6
Are these real(istic) numbers?

* General commentary
I think this proposal could significantly benefit from making it more
concise and cohesive. For example, section 15 seems really random, and
I had the feeling that it was a design doc mixed with implementation
notes for random details (that are at times *confidently* wrong and
therefore very hard to digest) that seemed out of place. It would be
easier to review if it was substantially shorter and more to the point
with a clearer organization of chapters, as well as adding citations
for many items to make it easier to trust/verify the claims, since
it's not easy to a-priori believe any given claim in the review for
reasons already shared by Christophe. Either links to items in the
literature or actual links to pieces of code would be good to save
time for the reader to not have to spend a lot of time just to figure
out whether something is made-up.





_______________________________________________
Sbcl-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/sbcl-devel

_______________________________________________
Sbcl-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/sbcl-devel