ClojureScript

memoization of js-dependency-index and get-upstream-deps needs knobs

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.7.145
  • Fix Version/s: 1.9.671
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

knobs should be exposed for more dynamic compilation environments like Figwheel which may desire to add dependencies to the classpath on the fly.

Activity

Hide
Bruce Hauman added a comment -

A patch that caches upstream dependencies in the compiler env.

Show
Bruce Hauman added a comment - A patch that caches upstream dependencies in the compiler env.
Hide
Bruce Hauman added a comment -

Actually I'm going to submit another patch that includes the memoize calls in js-deps.

Show
Bruce Hauman added a comment - Actually I'm going to submit another patch that includes the memoize calls in js-deps.
Hide
Bruce Hauman added a comment -

New patch that moves cljs.js-deps memoization to current env/compiler as well as get-upstream-deps.

Unfortunately there is a circular dep between cljs.env and cljs.js-deps, if we want to cache in env/compiler. I overcame this with a resolve.

Compile performance is either completely unchanged or slightly improved based on several test runs.

Show
Bruce Hauman added a comment - New patch that moves cljs.js-deps memoization to current env/compiler as well as get-upstream-deps. Unfortunately there is a circular dep between cljs.env and cljs.js-deps, if we want to cache in env/compiler. I overcame this with a resolve. Compile performance is either completely unchanged or slightly improved based on several test runs.
Hide
Bruce Hauman added a comment -

Hold off on this. Its not behaving as expected. Doesn't seem to be caching in certain situations.

Show
Bruce Hauman added a comment - Hold off on this. Its not behaving as expected. Doesn't seem to be caching in certain situations.
Hide
David Nolen added a comment -

Thanks for the update. This will definitely not land until after the pending REPL/piggieback release anyhow.

Show
David Nolen added a comment - Thanks for the update. This will definitely not land until after the pending REPL/piggieback release anyhow.
Hide
Bruce Hauman added a comment -

Yeah there is an obvious bug and a subtle one. Hopefully will finish it up soonish.

Show
Bruce Hauman added a comment - Yeah there is an obvious bug and a subtle one. Hopefully will finish it up soonish.
Hide
Bruce Hauman added a comment -

Alright, this latest patch works. There was a subtle memoizing nil value bug.

Show
Bruce Hauman added a comment - Alright, this latest patch works. There was a subtle memoizing nil value bug.
Hide
Mike Fikes added a comment -

Patch no longer applies.

Show
Mike Fikes added a comment - Patch no longer applies.

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated: