clj-commons/virgil

Integration with clojure.tools.namespace.repl workflow

aiba opened this issue · 3 comments

aiba commented

I write performance-sensitive clojure code, sometimes dropping into java, and virgil has been great for this. Thank you!

But frankly I don't want virgil to recompile on file saves. Would you want your clojure namespaces to recompile on save? For clojure I use clojure.tools.namespace.repl/refresh to manually trigger a refresh of necessary namespaces. I'd love for virgil to integrate with this.

The current story is to call virgil.compile/compile-all-java and then clojure.tools.namespace.repl/refresh-all as suggested by a user in #20. But recompiling all clojure namespaces is unecessarily slow (~30 seconds in my primary project). You really only need to recompile clojure namespaces that depend on changed java classes, and their dependents.

I think we can make this happen, and here's how it could work:

  • Keep some state containing:
    • The set of directories containing java code (analogous to clojure.tools.namespace.repl/refresh-dirs).
    • The result of the last java compilation.
  • Have a function virgil.repl/refresh that:
    • Calls virgil.compile/compile-all-java.
    • Checks the resultant bytecodes against the previous compilation to determine which classes have changed.
    • Scans clojure.tools.namespace.repl/refresh-dirs for clojure files that import any of the changed java classes.
    • Touches the dependent clojure files (changing their last-modified time) so that clojure.tools.namespace.repl/refresh will recompile them.

With this, anyone who wants to integrate java files into their refresh workflow could just call (virgil.repl/refresh) before calling (clojure.tools.namespace.repl/refresh).

I made a proof of concept and have been using it in my project successfully.

If you like the concept, I'd be happy to clean this up and submit it as a PR. This could just become a new namespace, virgil.repl. Virgil already depends on clojure.tools.namespace.

What do you think?

@aiba Please make a pull request! I would love to try it out.

I actually tried to do a minimal namespace refresh in earlier versions of Virgil, and it led to some problems (unreloaded namespaces that depend on protocols defined in the reloaded namespaces, for instance), which is why I ended up using the nuclear option.

However, I'm happy to create an alternate code path that has some caveats attached, if it doesn't touch the existing automatic reload functionality.

aiba commented

actually tried to do a minimal namespace refresh in earlier versions of Virgil, and it led to some problems (unreloaded namespaces that depend on protocols defined in the reloaded namespaces, for instance), which is why I ended up using the nuclear option.

I think the pasted gist above is handling this case by also reloading transitive dependents as well as immediate dependents of recompiled java classes. It's just leveraging clojure.tools.namespace to do it.

However, I'm happy to create an alternate code path that has some caveats attached, if it doesn't touch the existing automatic reload functionality.

Great to hear! I have learned some things using the above gist on my own project for the past few weeks. I will polish it up and submit a PR. It will not modify any existing virgil code.