ClojureScript

self calls do not optimize - regression

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 1.9.293
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

Activity

Hide
A. R added a comment -

This issue can be solve by changing the defn macro for the "simple" case and carrying the function name over to the function:

(core/list 'def (with-meta name m)
  (cons `fn (cons name fdecl)))

This isn't done in clojure because of: https://dev.clojure.org/jira/browse/CLJ-809

Though I don't think that's and issue in CLJS since we don't have real vars anyways and can't redefine {{def}}s.

Show
A. R added a comment - This issue can be solve by changing the defn macro for the "simple" case and carrying the function name over to the function:
(core/list 'def (with-meta name m)
  (cons `fn (cons name fdecl)))
This isn't done in clojure because of: https://dev.clojure.org/jira/browse/CLJ-809 Though I don't think that's and issue in CLJS since we don't have real vars anyways and can't redefine {{def}}s.
Hide
A. R added a comment - - edited

So the issue that the CLJ ticket has is emulated/shown below in CLJS:

(enable-console-print!)

(defn self-call-test
  [n]
  (prn "inner")
  (when (pos? n)
    (self-call-test (dec n))))

(let [orig self-call-test]
  (set! self-call-test
        (fn outer [n]
          (pr "outer")
          (orig n))))
(self-call-test 2)

(def self-call-test2
  (fn self-call-test2
       [n]
       (prn "inner")
       (when (pos? n)
         (self-call-test2 (dec n)))))

(let [orig self-call-test2]
  (set! self-call-test2
        (fn outer [n]
          (pr "outer")
          (orig n))))
(self-call-test2 2)

Output in with no optimizations:

"outer"
"inner"
"outer"
"inner"
"outer"
"inner"


"outer"
"inner"
"inner"
"inner"

So: It does seem this would also break the current behaviour, HOWEVER, the above with advance optimizations gives this:

"outer"
"inner"
"inner"
"inner"

*for both*. Given this, it seem better to not change behavior during advanced builds to avoid hard to track down production bugs for the users. Even if this is a slight deviation from CLJ behavior. Thoughts?

Show
A. R added a comment - - edited So the issue that the CLJ ticket has is emulated/shown below in CLJS:
(enable-console-print!)

(defn self-call-test
  [n]
  (prn "inner")
  (when (pos? n)
    (self-call-test (dec n))))

(let [orig self-call-test]
  (set! self-call-test
        (fn outer [n]
          (pr "outer")
          (orig n))))
(self-call-test 2)

(def self-call-test2
  (fn self-call-test2
       [n]
       (prn "inner")
       (when (pos? n)
         (self-call-test2 (dec n)))))

(let [orig self-call-test2]
  (set! self-call-test2
        (fn outer [n]
          (pr "outer")
          (orig n))))
(self-call-test2 2)
Output in with no optimizations:
"outer"
"inner"
"outer"
"inner"
"outer"
"inner"


"outer"
"inner"
"inner"
"inner"
So: It does seem this would also break the current behaviour, HOWEVER, the above with advance optimizations gives this:
"outer"
"inner"
"inner"
"inner"
*for both*. Given this, it seem better to not change behavior during advanced builds to avoid hard to track down production bugs for the users. Even if this is a slight deviation from CLJ behavior. Thoughts?
Hide
A. R added a comment -

Any thoughts on this? I can create a patch if that this change is ok. It could matter a bit (performance wise) since a few of the very low level data structure functions are recursive.

Show
A. R added a comment - Any thoughts on this? I can create a patch if that this change is ok. It could matter a bit (performance wise) since a few of the very low level data structure functions are recursive.

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated: