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

Integration with clojure.tools.namespace.repl workflow #22

Open
aiba opened this issue Mar 19, 2018 · 3 comments
Open

Integration with clojure.tools.namespace.repl workflow #22

aiba opened this issue Mar 19, 2018 · 3 comments

Comments

@aiba
Copy link
Contributor

aiba commented Mar 19, 2018

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?

@schmee
Copy link

schmee commented Apr 3, 2018

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

@ztellman
Copy link
Collaborator

ztellman commented Apr 3, 2018

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
Copy link
Contributor Author

aiba commented Apr 4, 2018

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.

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

No branches or pull requests

3 participants