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

Problems with defsfn #45

Closed
hsgrott opened this issue Nov 12, 2015 · 7 comments
Closed

Problems with defsfn #45

hsgrott opened this issue Nov 12, 2015 · 7 comments
Assignees
Labels

Comments

@hsgrott
Copy link

hsgrott commented Nov 12, 2015

I was running in the REPL some examples derived from this post:

http://yogthos.net/posts/2015-06-17-Using-Pulsar.html

Problem is, every time I redefined one of the suspendable functions (the ones created with defsfn), the -main method would hang, no matter which method was used (clojure.tools.namespace.repl/refresh, using CIDER commands, etc).

I've looked into the definition of the defsfn macro, and found the following:

(defmacro defsfn
  "Defines a suspendable function that can be used by a fiber or actor.
  Used exactly like `defn`"
  [& expr]
  `(do
     (defn ~@expr)
     (def ~(first expr) (suspendable! ~(first expr)))))

It seems that there is some kind of weird bug/interaction going on (or maybe I'm extremely unlucky :D). Anyways, here is an alternative version that seems to correct the problem:

(defmacro defsfnp
  "Walks through a 'defn' macroexpansion and wraps the generated fn* in
  a 'suspendable!' call." 
  [& expr]
  (-> (cons 'defn expr)
      walk/macroexpand-all
      zip/seq-zip
      zip/down
      zip/rightmost
      (zip/edit (fn [loc] `(suspendable! ~loc)))
      zip/root))

It might be a good idea to look up the actual fn* node instead of using zip/rightmost to make this macro more robust, but that is left as an exercise to the reader.

The code was tested in an Ubuntu 15.10 machine (Linux 4.2.0-18-generic #22-Ubuntu SMPx86_64 x86_64 x86_64 GNU/Linux) and a Mac using OSX El Capitan, both running JDK 1.8.0_66-b17, Clojure 1.7.0, pulsar 0.7.3 and quasar-core 0.7.3, with the same results.

Here is a gist with a sample project.clj, a sample test file and a bunch of tests adapted from the "def" tests in the main clojure repo:

https://gist.github.com/anonymous/dc9f501e8c52b3a7166c

I hope it helps.

@circlespainter
Copy link
Member

Thanks, even in a simple lein repl the "bugged" main seems to hang. But it looks like simply avoiding the re-definition of the var in the original defsfn macro solves the problem (and the Pulsar testsuite runs ok), at least with your test project. Can you confirm it works for you?

@hsgrott
Copy link
Author

hsgrott commented Nov 16, 2015

Hello @circlespainter,

Thank you for your answer.

I didn't understand your advice. When you say to avoid the re-definition of the var in the original defsfn macro, you mean removing the def part? I've tried that, but it didn't work properly. If the macro is defined like:

(defmacro defsfn
  "Defines a suspendable function that can be used by a fiber or actor.
  Used exactly like `defn`"
  [& expr]
  `(do
     (defn ~@expr))))

It would be a normal defn. But if the functions are defined with that macro (or directly with defn), similar problems arise. I've noticed that, in this particular case, spawn marks the function as suspendable when called with only one argument (like (spawn buggy-pong)), but will not do that when called with more than one argument (as in (spawn buggy-ping 3 a1)); you can test that by changing those functions to use defn and following it by calling suspendable? on them right after starting a fresh REPL. If you call (suspendable! buggy-ping) directly in the REPL after a fresh run, buggy-main will work as intended (or will not be buggy anymore :D).

The way things are right now, defsfn will work when you don't have a var redefinition (when you don't reload your code). It might pose no problems when deploying a new application or if you develop without using a REPL, but it also means that you can't use a REPL to experiment with Pulsar (at least in this specific configuration). I don't know if this is the intended behavior, a deliberate decision based on the design of Quasar/Pulsar (considering the instrumentation stuff), a bug (in spawn, in defsfn or both), or me doing things improperly.

I'm new to Pulsar, so I apologize if I'm getting something wrong or mixing things up.

Best regards.

@circlespainter
Copy link
Member

Sorry, I didn't explain thoroughly. I mean this:

(defmacro defsfn
  "Defines a suspendable function that can be used by a fiber or actor.
  Used exactly like `defn`"
  [& expr]
  `(do
     (defn ~@expr)
     (suspendable! ~(first expr))))

This is enough for me to make it work in the REPL and to fix your example while still having the testsuite fully ok.

@hsgrott
Copy link
Author

hsgrott commented Nov 17, 2015

Hi @circlespainter,

It's working now. Thank you very much.

There's only the situation with the spawn function. It seems that it doesn't mark a regular function as suspendable when called with more than one argument.

Best regards.

@circlespainter
Copy link
Member

My pleasure! Would you mind opening a separate issue for spawn?

@circlespainter circlespainter self-assigned this Nov 17, 2015
@hsgrott
Copy link
Author

hsgrott commented Nov 17, 2015

Done.

@circlespainter
Copy link
Member

Thanks!

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

No branches or pull requests

2 participants