Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

peg-grammar is not available in threads #1180

Open
zevv opened this issue Jun 4, 2023 · 5 comments
Open

peg-grammar is not available in threads #1180

zevv opened this issue Jun 4, 2023 · 5 comments

Comments

@zevv
Copy link
Contributor

zevv commented Jun 4, 2023

This snippet:

(ev/spawn-thread       
  (peg/match '{ :main :s* } "")) 

results in the following error:

error: grammar error in :s*, unknown rule
  in peg/match [src/core/peg.c] on line 1694
  in _spawn-thread [/tmp/t.janet] on line 2, column 3

Root cause is that the *peg-grammar* dynamic binding does not make it into the thread, just like any other dynamic binding.

@sogaiu
Copy link
Contributor

sogaiu commented Jun 4, 2023

Yes, although not reported in an issue AFAIK, this has been noted in discussions at one or more of the gitter / matrix channels.

A work-around I'd been using was putting:

(setdyn :peg-grammar default-peg-grammar)

in the definition of a function passed to ev/thread.

It seems to me that either it should be documented behavior (along with any other caveats [1] when spawning a thread) or the current behavior should be adjusted.


[1] I noticed that eval didn't give quite the same results in my testing, and I suppose it may be a related thing.

@zevv
Copy link
Contributor Author

zevv commented Jun 4, 2023

I must admit I have not looked deeply at the memory model for threads so I'm not sure what the original design decisions are, but I think conceptually it might make sense to have new threads inherit the parent's dynamic bindings?

@sogaiu
Copy link
Contributor

sogaiu commented Jun 4, 2023

For coro, the fiber that's created with fiber/new has :yi as part of its sigmask, and :i is "inherit the environment from the current fiber".

So if I do:

(ev/thread (coro (pp (peg/match {:main :s*} ""))))

I get:

@[]
nil

Currently ev/spawn-thread and ev/do-thread don't take any flags, but may be it would be nice if they did? Something like:

(ev/spawn-thread :i (peg/match {:main :s*} ""))

I don't suppose that would break any code -- an initial keyword in a function body presumably doesn't do much so I doubt many people have been using them for anything "usual" [1].


[1] I've seen use as what looks like labeling (presumably for human readers) as well as metadata that gets parsed by other code.

@bakpakin
Copy link
Member

bakpakin commented Jun 4, 2023

So the ev/thread function, which is what does all of the heavy lifting of setting up a new thread, takes some flags to determine what state to copy from the current thread into the new thread. I suppose it would make sense to copy over dynamic bindings as an option if specified - however, copying over the entire environment table as returned by (curenv), table proto types and all, is prohibitively expensive.

I noticed that eval didn't give quite the same results in my testing, and I suppose it may be a related thing.

Yes, eval needs to have available function definitions in it's environment, so in order to see all available functions they would need to all be copied to the new thread.

For efficiency, the default behavior is to copy nothing that isn't needed.

@bakpakin
Copy link
Member

bakpakin commented Jun 4, 2023

For completeness, as a work around I should mention that ev/thread can take a fiber argument as well as a function argument - while this doesn't help if you are using the built-in ev/do-thread and ev/spawn-thread macros, you could create your own subroutine to wrap a closure or function body in a fiber that captures all of the state you were interested in.

For example

(defn wrap-closure-for-thread
  "Wrap a closure in a fiber that captures the current dynamic bindings state."
  [f]
  (def env @{})
  (each k (all-dynamics) (put env k (dyn k)))
  (fiber/setenv (fiber/new f :t) env))
  
# Use with ev/thread
(ev/thread (wrap-closure-for-thread (fn [&] (pp (dyn *peg-grammar*)))))

Edit: @sogaiu already mentioned this but above is more explicit about what is happening and how to capture precisely the state that you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants