<< Back to previous view

[CLJS-1501] Add :parallel-build support to REPLs Created: 05/Dec/15  Updated: 27/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Task Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The :parallel-build option does not currently work in REPLs due to the implementation of cljs.repl/load-namespace






[CLJS-1300] REPLs do no write out updated deps.js when compiling files Created: 05/Jun/15  Updated: 27/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File cljs-1300.patch    
Patch: Code

 Description   

For example a user may edit a file including a new dependency. This will work at the REPL but if a browser refresh is made the emitted goog.require will fail due to the initial deps.js file being stale.



 Comments   
Comment by ewen grosjean [ 05/Dec/15 4:15 PM ]

load-file is broken into 4 sub-functions:
repl-compile-cljs: compile the cljs file beeing loaded
repl-cljs-on-disk: ensures all dependencies are on disk
refresh-cljs-deps: refreshes the cljs_deps.js file
repl-eval-compiled: eval the compiled file

Comment by David Nolen [ 05/Dec/15 9:02 PM ]

Thanks will review.

Comment by Mike Fikes [ 31/Jan/16 3:25 PM ]

cljs-1300.patch no longer applies on master





[CLJS-968] Metadata on function literal inside of a let produces invalid Javascript Created: 07/Jan/15  Updated: 27/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Defect Priority: Major
Reporter: Bobby Eickhoff Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: bug
Environment:

Originally found with [org.clojure/clojurescript "0.0-2496"]
Still reproducible with the latest cljsc (b5e9a5116259fc9f201bee4b9c6564f35306f9a5)



 Description   

Here is a minimal test case that produces the invalid Javascript:

(defn f []
  (let [a 0]
    ^{"meta" "data"}
    (fn [] true)))

The compiled Javascript includes the invalid token sequence "return return". (Per Chrome: Uncaught SyntaxError: Unexpected token return)

The problem does not occur if the metadata applies to a map literal instead of a function literal.
The problem only occurs when the function and metadata are inside of a let.



 Comments   
Comment by Bobby Eickhoff [ 07/Jan/15 9:45 PM ]

I forgot to try with-meta. Using with-meta does not produce this syntax error, so it's only a problem with the reader macro for metadata.

Comment by David Nolen [ 08/Jan/15 7:41 AM ]

Any quick thoughts about this one Nicola? Quite possibly a compiler issue on the CLJS side.

Comment by Nicola Mometto [ 08/Jan/15 8:07 AM ]

David, I understand why this happens but I don't know enough about how cljs's js emission to propose a fix.
The issue is that with this commit: https://github.com/clojure/clojurescript/commit/d54defd32d6c5ffcf6b0698072184fe8ccecc93a the following scenario is possible:

{:op :meta
 :env {:context :return}
 :expr {:op :fn
        :env {:context :expr}
        :methods [{:op :fn-method 
                   :env {:context :return} ..}]
        ..}
 ..}

i.e. analyze-wrap-meta changes the context of the :fn node to :expr but keeps the context of the :fn-methods to :return.

This causes both
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L575-L576
and
https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L488 (https://github.com/clojure/clojurescript/blob/master/src/clj/cljs/compiler.clj#L233)

to be true and emit a "return".

Comment by David Nolen [ 06/May/15 7:15 PM ]

Hrm, it appears analyze-wrap-meta may need to defer to a helper to change the :context of the given AST node.

Comment by Herwig Hochleitner [ 11/Dec/15 10:52 AM ]

I just randomly ran into this, when upgrading an old project. There is also a duplicate already: http://dev.clojure.org/jira/browse/CLJS-1482

Comment by Jonathan Chu [ 28/Jan/16 6:19 PM ]

This issue occurs for me even without a let.

(fn []
  ^{"meta" "data"}
  (fn [] true))

gives me

#object[SyntaxError SyntaxError: Unexpected token return]




[CLJS-719] this-as behaves incorrectly in "scoping function" Created: 07/Dec/13  Updated: 27/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Defect Priority: Major
Reporter: Kevin Marolt Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None


 Description   

When a this-as expression gets put in a "scoping function", e.g. in a let-binding, the value bound via this-as refers to the scoping function, and not to the outer scope.

Example:

(def foo
  (js-obj
    "bar" "baz"
    "getBarRight" (fn [] (this-as self (.-bar self)))
    "getBarWrong" (fn []
                    (let [bar (this-as self (.-bar self))]
                      bar))))
     
(.log js/console (.getBarRight foo)) ;; => "baz"
(.log js/console (.getBarWrong foo)) ;; => undefined

Whereas foo.getBarRight expands to something like

function() {
  var self = this; // this refers to foo
  return self.bar; // returns "bar"
}

foo.getBarWrong on the other hand expands to

function() {
  var bar = function() {
    var self = this; // this refers to enclosing function
    return self.bar; // returns undefined
  }();
  return bar; // returns undefined
}





[CLJS-1896] Externs file validation Created: 23/Jan/17  Updated: 27/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.655
Fix Version/s: 1.9.671

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Invalid externs file will fail silently and not provide the expected externs inference warnings. We should try to catch such issues when we parse the file and throw an exception.






[CLJS-2247] Warn when overwriting protocol method Created: 15/Jul/17  Updated: 16/Jul/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.671
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: Text File CLJS-2247.patch    
Patch: Code and Test

 Description   

Warn when a protocol overwrites a method in another protocol.

Observe the warning in this Clojure REPL session:

user=> (defprotocol IAlpha
  #_=>   (-foo [this]))
IAlpha
user=> (defrecord Alpha []
  #_=>   IAlpha
  #_=>  (-foo [this] :alpha))
user.Alpha
user=> (defprotocol IBeta
  #_=>   (-foo [this]))
Warning: protocol #'user/IBeta is overwriting method -foo of protocol IAlpha
IBeta
user=> (defrecord Beta []
  #_=>   IBeta
  #_=>   (-foo [this] :beta))
user.Beta
user=> (-foo (->Alpha))

IllegalArgumentException No implementation of method: :-foo of protocol: #'user/IBeta found for class: user.Alpha  clojure.core/-cache-protocol-fn (core_deftype.clj:568)

Here is the same in ClojureScript:

To quit, type: :cljs/quit
cljs.user=> (defprotocol IAlpha
(-foo [this]))
nil
cljs.user=> (defrecord Alpha []
IAlpha
(-foo [this] :alpha))
cljs.user/Alpha
cljs.user=> (defprotocol IBeta
(-foo [this]))
nil
cljs.user=> (defrecord Beta []
IBeta
(-foo [this] :beta))
cljs.user/Beta
cljs.user=> (-foo (->Alpha))
repl:13
throw e__5612__auto__;
^

Error: No protocol method IBeta.-foo defined for type : [object Object]
    at cljs$core$missing_protocol (/Users/mfikes/.cljs_node_repl/.cljs_node_repl/cljs/core.js:317:9)
    at cljs$user$_foo (repl:25:34)
    at repl:1:94
    at repl:9:3
    at repl:14:4
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at Object.runInThisContext (vm.js:116:38)
    at Domain.<anonymous> ([stdin]:50:34)
    at Domain.run (domain.js:242:14)
    at Socket.<anonymous> ([stdin]:49:25)

Note that they both result in the same runtime behavior, but Clojure emits a nice diagnostic when IBeta is defined.



 Comments   
Comment by Mike Fikes [ 16/Jul/17 9:28 AM ]

This is an interesting one: While the attached patch produces nice warnings, even without it the :redef-in-file warning triggers if the forms involved happen to reside in files, and in that case you'd get two slightly different warnings being emitted:

WARNING: Protocol IDelta is overwriting method -bar of protocol IGamma in file /Users/mfikes/Desktop/src/foo/core.cljc
WARNING: -bar at line 6 is being replaced at line 7 /Users/mfikes/Desktop/src/foo/core.cljc

Each of the warnings above has their own strengths; but having both seems to be a negative. Here are my initial thoughts on variants:

  1. Leave both: Could be perceived to be noisy, albeit useful if it occurs.
  2. Have the warning disabled, but enabled for the REPL: Odd to have a warning that only happens while in the REPL (and you can still get two when loading files)
  3. Have the warning only be emitted for REPL-entered forms: This perhaps directly addresses the issue: (No double warnings, useful for people learning the language). But this seems even odder to have a warning limited to REPL-entered forms.
  4. Figure out a way to suppress the :redef-in-file warning in this case: Maybe cleaner.
  5. Same as above, but find a way to get the line numbers into the "overwrite" warning: Perhaps cleanest, but difficult to do?
  6. Do nothing, decline the ticket: Perhaps this leaves users learning the language at a loss if they encounter this while in the REPL where redefining forms is "normal", but not overwriting via protocol definitions.




[CLJS-2002] Don't throw when no *print-fn* is set Created: 07/Apr/17  Updated: 18/Aug/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.671
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: errors, warnings

Attachments: Text File CLJS-2002.patch    
Approval: Vetted

 Description   

Currently calling (enable-console-print!) causes a bunch of code to be retained in :advanced mode even if you never print.

While that is not ideal it doesn't cause runtime errors. Not calling it and trying to print however will throw an exception which will potentially break your app.

No *print-fn* fn set for evaluation environment

So we end up in a no-win situation for :advanced builds where a "forgotten" prn may break your app in production or "maybe" bloating your file size by retaining all the print-related things.

I think the no-print-fn condition should never throw, maybe just try to write a warning using console.log. Or just dropping the prn altogether.



 Comments   
Comment by David Nolen [ 13/Jul/17 8:29 PM ]

Let's move the old behavior to `string-print` only.

Comment by Mike Fikes [ 13/Jul/17 9:01 PM ]

The attached CLJS-2002.patch moves the throw to string-print and using nil as the init for the two Vars solves CLJS-2231. But it doesn't address the ticket as written: Leaving an inadvertent prn in your code leads to a call to string-print which throws.

Comment by David Nolen [ 13/Jul/17 10:06 PM ]

https://github.com/clojure/clojurescript/commit/797e247fbef676544060a57da995f058db061f37 partially addresses this issue. Keeping this open and moving to lower priority, we should revisit.





[CLJS-1702] Warning when using private vars Created: 07/Jul/16  Updated: 18/Aug/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.671
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: warnings

Approval: Vetted

 Description   

Currently no warning or error of any kind is given. Throwing an error and forcing users to go through vars is somewhat less attractive since vars dump information like file, line, etc. A warning would be a simple way to flag users that they are treading into dangerous territory. Downstream tooling error handling can make it a hard error if they like.






[CLJS-1701] cljs.spec impact on :advanced builds Created: 07/Jul/16  Updated: 07/Jul/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Thomas Heller Assignee: David Nolen
Resolution: Unresolved Votes: 2
Labels: None


 Description   

Investigate the impact of cljs.spec on :advanced builds.

Currently all specs are kept in the (private) cljs.spec/registry-ref atom. This atom is not understood by the Closure Compiler and cannot be eliminated as dead code. So even if specs are not used in "production" they still bloat the generated JS size. Some specs may be used at runtime and cannot not be removed, the gen parts however are probably never required in :advanced builds and should be omitted somehow.

In a test build (with 1.9.93) this adds 11kb (102kb vs 91kb) as soon as cljs.spec is :require'd somewhere and goes up with each defined spec.






[CLJS-868] no arity warnings on recursive calls Created: 03/Oct/14  Updated: 05/Oct/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File cljs_868_14_Nov_2015.md     Text File cljs_868_14_Nov_2015.patch    
Patch: Code

 Description   

If a function recursively invokes itself within its own body the invoke will not be checked for arity mismatch.



 Comments   
Comment by Samuel Miller [ 10/Aug/15 10:06 PM ]

Took some time to look at this issue. Originally thought "Do what loop/recur does" but that does not take into account multi-arity. It seems like maybe the best option is to somehow use the second pass of the analyze(analyze-fn-methods-pass2). The entire information about the function is present and the warning section of the code gets triggered but because of no-warn is ignored. Any other ideas for a solution to this?

Comment by Samuel Miller [ 14/Nov/15 7:47 PM ]

So I am looking for feed back on this patch and I will try to explain the reasoning for each section.

The issue is that a function only knows about it's arity after it has been parsed once.
So we need to check arity issues on the second pass

First off, added two new variables.
-activate-second-pass-warnings:Boolean Basically if you want to have second-pass warnings turned on
-second-pass-cljs-warnings:Set Right now we only have :fn-arity but I figure might as well make it generic.

So first up if the modifications to the analyze-fn-methods-pass2 function.
Instead of using no-warn marco here we have some new functionality.
The goal is to turn everything off except the second-pass warnings

So if activate-second-pass-warnings is false just use no-warn else it will use the new section of code.

The default-warning-handler was also modified. After checking if a warning is on, it checks if the warning is a second-pass warning and
if that warning can now be activated. If activate-second-pass-warnings is false AND a warning is still on that implies it is a second pass warning
in the second pass so we activate it.

Also I tried to keep all modifications in cljs.analyzer.

Originally I had the cljs-warnings :fn-arity to false and it would only be turned on in the second pass.
However the repl section just sets everything to true (and turns off select parts like ns errors).
So I decided to not touch those sections and instead keep how other files interface with the analyzer the same.

Comment by Samuel Miller [ 16/Nov/15 10:58 PM ]

Just realized that I have the patch marked as .md instead of .patch





[CLJS-1278] Asserts still fail while :require-ing .js file (either in :libs or in :source-paths) (same as CLJS-1196) Created: 20/May/15  Updated: 14/Jul/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 0.0-3269
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Michal Till Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File cljs_1278.patch    

 Description   

Following on CLJS-1196, I can't get it to work.

In version 0.0-3264 lein-cljsbuild crashed on weird eception `Caused by: java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil"` but the current version 0.0-3269 gives the same failed assertion as previously.

I've put up a sample project to illustrate the issue.

Steps to reproduce:

`git clone https://github.com/tillda/stackone`
`cd stackone`
`git checkout 537e5c69b844bc53c159e85cafc24310543cc918`
`lein clean && lein cljsbuild once temp`

Expected behaviour: cljs compiled successfully with src/vendor/client/closure.js and env/stackone/helpersjs.js being included.

Actual behaviour:

```
Compiling "resources/public/lein-cljsbuild-temp/dev-mode-deps.js" failed.
Exception in thread "main" java.lang.AssertionError: Assert failed: (or (file? x) (url? x) (string? x)), compiling/private/var/folders/ym/l2qxd7l97kzfzftrdpqsclm40000gn/T/form-init3642888309490821030.clj:1:125)
at clojure.lang.Compiler.load(Compiler.java:7249)
at clojure.lang.Compiler.loadFile(Compiler.java:7175)
at clojure.main$load_script.invoke(main.clj:275)
at clojure.main$init_opt.invoke(main.clj:280)
at clojure.main$initialize.invoke(main.clj:308)
at clojure.main$null_opt.invoke(main.clj:343)
at clojure.main$main.doInvoke(main.clj:421)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:383)
at clojure.lang.AFn.applyToHelper(AFn.java:156)
at clojure.lang.Var.applyTo(Var.java:700)
at clojure.main.main(main.java:37)
Caused by: java.lang.AssertionError: Assert failed: (or (file? x) (url? x) (string? x))
at cljs.util$ext.invoke(util.cljc:115)
at cljs.closure$source_on_disk.invoke(closure.clj:1206)
at cljs.closure$output_unoptimized$fn__3708.invoke(closure.clj:1235)
at clojure.core$map$fn__4551.invoke(core.clj:2622)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$filter$fn__4578.invoke(core.clj:2677)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$map$fn__4551.invoke(core.clj:2614)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.Cons.next(Cons.java:39)
at clojure.lang.RT.next(RT.java:674)
at clojure.core$next__4110.invoke(core.clj:64)
at clojure.core$str$fn__4186.invoke(core.clj:528)
at clojure.core$str.doInvoke(core.clj:526)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invoke(core.clj:628)
at cljs.closure$deps_file.invoke(closure.clj:1040)
at cljs.closure$output_deps_file.invoke(closure.clj:1060)
at cljs.closure$output_unoptimized.doInvoke(closure.clj:1243)
at clojure.lang.RestFn.applyTo(RestFn.java:139)
at clojure.core$apply.invoke(core.clj:630)
at cljs.closure$build.invoke(closure.clj:1514)
at cljs.closure$build.invoke(closure.clj:1426)
at cljsbuild.compiler$compile_cljs$fn__3884.invoke(compiler.clj:81)
at cljsbuild.compiler$compile_cljs.invoke(compiler.clj:80)
at cljsbuild.compiler$run_compiler.invoke(compiler.clj:187)
at user$eval4018$iter_40544058$fn4059$fn_4077.invoke(form-init3642888309490821030.clj:1)
at user$eval4018$iter_40544058$fn_4059.invoke(form-init3642888309490821030.clj:1)
at clojure.lang.LazySeq.sval(LazySeq.java:40)
at clojure.lang.LazySeq.seq(LazySeq.java:49)
at clojure.lang.RT.seq(RT.java:507)
at clojure.core$seq__4126.invoke(core.clj:135)
at clojure.core$dorun.invoke(core.clj:3007)
at clojure.core$doall.invoke(core.clj:3023)
at user$eval4018.invoke(form-init3642888309490821030.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:6792)
at clojure.lang.Compiler.eval(Compiler.java:6782)
at clojure.lang.Compiler.load(Compiler.java:7237)
... 11 more
Subprocess failed
```



 Comments   
Comment by David Nolen [ 20/May/15 10:21 AM ]

This issue is in danger of being closed. Please supply minimal steps to reproduce that do not involve anything other than the ClojureScript compiler. We no longer have time to wade through the indirection introduced by cljsbuild or any other downstream tooling. Thanks.

Comment by Michal Till [ 20/May/15 11:14 AM ]

@David Nolen: I have created a failing minimal testcase based on the Quick Start document. Here it is: https://github.com/tillda/cljs-testcase/

Comment by David Nolen [ 20/May/15 11:27 AM ]

Michal the failing example is not correct. You are not supplying any :libs option.

Comment by Michal Till [ 20/May/15 11:45 AM ]

Ah! Thank you very much! This additional issue was therefore my error. Now it seems to work even in my "big" example.

However it would be cool if there was a meaningful error message stating that a file path can't be resolved. If one is not an expert in the cljs compiler this is almost impossible to figure out. After all the error message in the CLJS-1196 issue and in this wrongfully reported one are exactly the same.

You may close this issue.

Comment by David Nolen [ 20/May/15 11:55 AM ]

We'll leave it open for the improving the error message.

Comment by Sebastian Bensusan [ 22/May/15 7:16 AM ]

Added the check in cljs.closure/source-on-disk where there is info for the error message.

For the supplied case, the error message is:

java.lang.IllegalArgumentException: The file file:/home/carlos/Playground/cljs-testcase/src/hello_world/closure.js 
lacks an associated source file. If it is a JavaScript library please add it to :libs}}

If a different wording or location of the check is needed, I'll submit a new patch with corrections.

Notes:

  • Changed:(:provides js) to (-provides js) in order to be consistent with IJavaScript.
  • cljs.clojure/source-on-disk takes a js argument that should satisfy with IJavaScript and ISourceMap if :source-map is enabled but the implementation is hardcoded to maps because :source-map and :source-url are used instead of ISourceMap methods -source-map and -source-url. I propose to extend PersistentMap and PersistentArrayMap to ISourceMap to make source-on-disk compliant with both protocols.




[CLJS-404] Automate Browser REPL testing Created: 23/Oct/12  Updated: 05/Jan/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It's worth investigating Selenium, PhantomJS, etc. as solutions to sanity check the Browser REPL when we run the other tests.



 Comments   
Comment by Robert Krahn [ 22/Dec/14 1:22 PM ]

An attempt: https://github.com/clojure/clojurescript/pull/42

Comment by David Nolen [ 24/Dec/14 8:57 AM ]

This looks like an interesting patch, thanks!

Comment by Robert Krahn [ 26/Dec/14 10:57 AM ]

I'll post a patch here, first I'll investigate the load-file issue, though.





[CLJS-374] satisfies? produces strange code when the protocol is not in the fast-path list Created: 06/Sep/12  Updated: 05/Jan/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None





[CLJS-1494] turn cljs.core/*assert* into a goog-define Created: 25/Nov/15  Updated: 22/Feb/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Thomas Heller Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File goog-define-assert.patch    
Patch: Code

 Description   

This patch turns the cljs.core/*assert* boolean into a goog.define and also checks *assert* at runtime (instead of only at compile-time).

The closure define option allows the closure compiler to eliminate asserts in :advanced, while :none builds can keep the asserts. This is one of the few remaining issues that prevent :advanced builds to re-use :none compiled (cached) files.

:elide-asserts is unaffected to keep this as simple as possible, but could be built on top of the goog.define instead of actually affecting the compiled output.



 Comments   
Comment by Mike Fikes [ 20/Feb/16 8:02 AM ]

Patch no longer applies, probably owing to CLJS-970.

Comment by Thomas Heller [ 22/Feb/16 5:08 AM ]

There was one more issue I discovered with my approach. My goal was to enable the Closure Compiler to eliminate the asserts when using :advanced compilation. This works perfectly fine with using a goog.define for *assert* but the compiler will complain if you try to adjust the define later since goog.define vars are not allowed to be adjusted at runtime.

(binding [*assert* false]
  (something-that-asserts))

This works in CLJ but not in CLJS since *assert* is only checked at compile time. If compiled with :elide-asserts true you can't bind assert to true either since the code no longer exists.

So some compromise must be made either way, the best solution IMHO would be to have a goog.define which lets the compiler decide whether to eliminate the asserts or not, independent from the *assert* and then moving the assert check itself into js instead of the compiler.

Happy to write the patch if interested.





[CLJS-1474] Error if reserved symbol is defined Created: 21/Oct/15  Updated: 31/Jul/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Martin Klepsch Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File CLJS-1474.patch    
Patch: Code and Test

 Description   

Currently a definition like

(defn set! [] ...)

will not cause any warning. Any usage of it (without :as namespace aliasing) however will not use the defined var but the set! special form.

A warning seems appropriate.



 Comments   
Comment by António Nuno Monteiro [ 30/Jul/16 1:19 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 31/Jul/16 2:09 PM ]

I know David suggested that a hard error is probably the right thing to do for this one, but one consequence is that the cljs.spec/def macro cannot be defined in bootstrap with this change. (I haven't investigated thoroughly, but this may simply be the result of macros being processed as ClojureScript in bootstrap, and thus being subject to this new guard.)

Regardless of the root cause, you'll see this if you try to run script/test-self-parity:

#error {:message "Could not eval cljs.spec", :data {:tag :cljs/analysis-error}, :cause #error {:message "Can't def special form at line 51 ", :data {:file nil, :line 51, :column 1, :tag :cljs/analysis-error}}}

For reference: Line 51 currently points at the def macro: https://github.com/clojure/clojurescript/blob/e2db5d9ff8cb6a099ebc2a8cd379385bf4649b38/src/main/cljs/cljs/spec.cljc#L51





[CLJS-1627] jsdoc parsing fails to recognize union types, breaking resolution Created: 18/Apr/16  Updated: 02/Sep/16

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.228
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Patrick Killean Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: patch

Attachments: Text File CLJS-1627-4.patch     Text File CLJS-1627-5.patch    
Patch: Code and Test

 Description   

The Closure Spec For Union Types states that parentheses are necessary for union type expressions. Trying this ...

(defn foo
  "@param {(IBar|IMap)} x"
  [x] 
  ...)

Raises a Closure Error :

...ERROR - Bad type annotation. expected closing }
* @param {user.(IBar|user.IMap)}

This is because comp/resolve-types treats the parentheses as a part of the type tokens and incorrect var resolution occurs as a result. In addition, the compiler emits multiple resolved types separated by "|" characters but does not enclose them in parentheses to create a valid union type.



 Comments   
Comment by Patrick Killean [ 18/Apr/16 4:36 PM ]

This patch includes:

  • comp/resolve-types now removes parentheses when present and emits them when >1 type is detected. This makes parenthesis use optional and existing code remains unbroken (with the added benefit that it may work now)
  • changes to comp/resolve-type
    1. checks for js globals like document or window which are recognized by closure
    2. allows dot.delimited.forms to pass through so we can use types defined in externs and avoid unnecessary resolution
    3. uses ana/resolve-existing-var with a "unresolved jsdoc type" warning
    4. checks if a resolved var is a protocol and warns otherwise. This is more informative than Closure's standard unrecognized type error
  • a test for comp/resolve-types
Comment by David Nolen [ 21/Apr/16 12:45 PM ]

Thanks will try to look more closely at this tomorrow.

Comment by David Nolen [ 23/Apr/16 2:03 PM ]

The patch is getting there, please remove the `js-doc-type` meta stuff. Just extend the signature of resolve-existing-var to take an additional parameter - the confirm-var-exists handler.

Comment by Patrick Killean [ 09/May/16 4:58 PM ]

CLJS-1627-1.patch:
resolve-existing-var now has an additional arity that accepts a missing-var handler passed to confirm-existing-var

Comment by Patrick Killean [ 10/May/16 6:16 AM ]

This has revealed a problem where deftype + defrecord using Object protocols emit resolved names when really they shouldn't. For example : "@implements {cljs.core.async.impl.timers.Object}" --> Bad Type Annotation

Since Object is a special case simply excluding it from the comments should fix it. Another patch incoming

Comment by Patrick Killean [ 10/May/16 7:42 AM ]

CLJS-1627-2.patch:
The emit* methods for deftype and defrecord now filter out Object protocols.

This produced an interesting result! With no more bad type annotations, static analysis can now proceed... and it has alot to say. Theres all kinds of info now about arity discrepencies (particularly cljs.core.IndexedSeq), type mismatches, and more. It even includes a type coverage percentage. Lots to parse here but very cool.

Comment by Patrick Killean [ 18/May/16 4:26 PM ]

CLJS-1627-3.patch:

  • fix require extern
  • add type application support for Array & Object
  • GC likes uppercase for Object & Array, lowercase for string, number.
  • support for explicit nullable types, variable typed arg
  • function type context modifiers this + new

Missing is the GC 'record type' . It also may be useful to fill out the node externs for common types

Comment by Patrick Killean [ 20/May/16 11:42 AM ]

CLJS-1627-4.patch:

  • fix a few problems in last patch
  • add record type support. Everything here should be covered
Comment by Patrick Killean [ 02/Sep/16 8:21 AM ]

update patch





[CLJS-1924] The compiler cannot infer the target type of "(. RecordName -prototype)" expressions Created: 01/Feb/17  Updated: 01/Feb/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: compiler, extern


 Description   

Repro:

Place

(set! *warn-on-infer* true)

(defrecord Foo [])

anywhere in your source files, compile with :infern-externs true.

Expected:

Multiple warnings like:

  • WARNING: Cannot infer target type in expression (. Foo -prototype)
  • WARNING: Cannot infer target type in expression (. other__8838__auto__ -constructor)
  • WARNING: Cannot infer target type in expression (. user/Foo -getBasis)

There are also warnings for (. cljs.core/List -EMPTY), but this might be unrelated.






[CLJS-1970] Cannot infer target type for list/vector expressions Created: 08/Mar/17  Updated: 10/Mar/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Daniel Janus Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Ubuntu 16.04.2, Oracle JRE 8



 Description   

With

(set! *warn-on-infer* true)
enabled, attempting to compile functions like:

(defn foo [] (list))
(defn bar [] (vector))

results in a warning:

WARNING: Cannot infer target type in expression (. cljs.core/List -EMPTY) at line 2427 src/cljs/discann/core.cljs

WARNING: Cannot infer target type in expression (. cljs.core/PersistentVector -EMPTY) at line 2435 src/cljs/discann/core.cljs

The line number reported is totally unrelated to the line of code where the problematic fn appears.

Affects 1.9.456 and 1.9.494.






[CLJS-1975] Perf: Compare f with nil in Delay impl Created: 11/Mar/17  Updated: 11/Mar/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-1975.patch    
Patch: Code

 Description   

When a Delay has been realized, f is set to nil. We can avoid truth_ and not calls and directly compare with nil for a minor perf boost.

In script/noderepljs, this leads to these

(simple-benchmark [x (delay 1)] @x 1e9)
(simple-benchmark [x (delay 1)] (realized? x) 1e9)
(simple-benchmark [x (doto (delay 1) deref)] (realized? x) 1e9)

speeding up by 6%, 11% and 9%, respectively.






[CLJS-1164] quot and rem are inefficient Created: 24/Mar/15  Updated: 25/May/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Francis Avila Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: math

Attachments: Text File CLJS-1164-1.patch     Text File cljs-1164.patch    
Patch: Code and Test

 Description   

The implementation of the quot and rem functions are needlessly complicated. Currently they are:

(defn quot [n d] (fix (/ (- n (js-mod n d)) d)))
(defn rem [n d] (- n (* d (quot n d))))

However all numbers in js are doubles already, so all this is unnecessary:

(defn quot [n d] (fix (/ n d)))
(defn rem [n d] (js-mod n d)))

Notice that "rem" is simply js-mod, and I'm not sure why no one noticed this before. I keep js-mod for now since a lot of code uses it, and if cljs ever grows a number tower the distinction may be important.

Patch attached, which also:

  • Creates a macro version of quot and rem.
  • Updates documentation for quot, rem, js-mod and mod for clarity.
  • Implement fix (private function to round to zero) with ES6 Math.trunc() if available.

Existing quot and rem tests pass, although there could be some better tests of edge cases (negative decimal num or div, NaN and +-Infinity args).



 Comments   
Comment by Francis Avila [ 24/Mar/15 12:27 PM ]

Better tests found rounding errors in my updated rem, which should stay as-is. (Not simply js-mod after all! Seems to round args first? Not obvious from the spec.) Changed quot however is correct and introduces less error than the current one. Will update patch and tests when I get a chance.

Comment by Francis Avila [ 29/Mar/15 12:39 AM ]

Working patch with tests attached. Tests expanded to cover floating-point cases. rem is now fundamentally the same as master (was more accurate than js-mod!!), but returns results consistent with js-mod for non-finite args or zero divisor.

Comment by Mike Fikes [ 31/Jan/16 3:23 PM ]

cljs-1164.patch no longer applies on master

Comment by Andrea Richiardi [ 14/Feb/16 9:02 PM ]

Patch now applies. I only tested with Nashorn:

V8_HOME not set, skipping V8 tests
SPIDERMONKEY_HOME not set, skipping SpiderMonkey tests
JSC_HOME not set, skipping JavaScriptCore tests
Testing with Nashorn

...

Ran 185 tests containing 17195 assertions.
0 failures, 0 errors.
Tested with 1 out of 4 possible js targets
Comment by Andrea Richiardi [ 14/Feb/16 9:02 PM ]

Patch cleaned up

Comment by Mike Fikes [ 20/Feb/16 10:11 PM ]

Successfully ran Andrea's update to Francis's patch through V8, SpiderMonkey, JavaScriptCore, and Nashorn unit tests.

I also manually ran some of the unit tests in bootstrapped ClojureScript built with the patch.

LGTM.

Comment by Mike Fikes [ 20/Feb/16 10:23 PM ]

Since this is a low-level numerics update, also ran the unit tests through ChackraCore (successfully).

Comment by Mike Fikes [ 25/May/17 7:29 PM ]

CLJS-1164-1.patch no longer applies to master





[CLJS-1587] Duplicate keys via quoting Created: 24/Feb/16  Updated: 15/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.228
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File CLJS-1587.diff    
Patch: Code and Test

 Description   

For

#{1 '1}

you get

#{1 1}


 Comments   
Comment by Peter Schuck [ 03/Mar/16 10:01 PM ]

This happens for the has-set macro and the hash-set literal. Here's what I get from the repl

cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4 5)
#{1 2 3 4 5}
cljs.user=> (hash-set 1 '1 2 '2 3 '3 4 '4)
#{1 1 2 2 3 3 4 4}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4}
#{2 1 4 4 3 2 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5}
#{2 1 4 4 3 2 5 1 3}
cljs.user=> #{ 1 '1 2 '2 3 '3 4 '4 5 '5}
#{2 5 1 4 4 3 2 5 1 3}
cljs.user=> (apply hash-set [1 '1 2 '2 3 '3 4 '4])
#{1 2 3 4}

Calling hash-set as a function gives the correct results. The hash-set macro gives the incorrect results until we have more then 8 elements and uses the fromArray method on PersistentHashSet to build the set instead of creating a literal PersistentArrayMap for the set. The literal notation is incorrect no matter how many elements there are.

Comment by Rohit Aggarwal [ 15/Jun/16 8:02 AM ]

The underlying problem for both is the same in that a PersistentHashSet is being created directly using a PersistentArrayMap where the keys are the elements from the provided sequence. It manifests itself in two places though.

Comment by Rohit Aggarwal [ 15/Jun/16 9:32 AM ]

I've taken the approach that if we see a quoted constant then don't create the PersistentHashSet directly and instead go via the fromArray function.

Patch has the fix and a test.

Comment by Mike Fikes [ 13/Jun/17 9:40 PM ]

Attached patch no longer applies to master.

Comment by A. R [ 14/Jun/17 1:43 AM ]

It'd be nice if this patch/ticket also included the following case:

(hash-set "a" \a)
Comment by A. R [ 14/Jun/17 10:50 AM ]

Should we increase the scope of this ticket? The same issue exists for maps:

{'0 "a", 0 "b"}
{\a "a", "a" "b"}

I think one possible solution that solves both, quoting and the char/string, could be to to call emit-str in cljs.compiler on the keys/set-members and only then check for uniqueness. Not sure that's a good idea though.

Doesn't solve the hash-set, array-map macros.

Edit: Related ticket: CLJS-2087

Comment by David Nolen [ 14/Jun/17 1:45 PM ]

Increasing the scope of tickets is not desirable. Please move to a separate issue and cross-reference thanks.





[CLJS-2004] Minor fix for test-simple script Created: 10/Apr/17  Updated: 14/May/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Dejan Josifovic Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: clojurescript, script, test
Environment:

Lubuntu 16.10


Attachments: Text File CLJS-2004.patch    
Patch: Code

 Description   

On Ubuntu based Linux distributions $[] doesn't work.
Output is: Tested with $[ran+1] out of 4 possible js targets
and should be: Tested with 4 out of 4 possible js targets

As in CLJS-929 (for test script), $(()) will work for ash, dash, bash, and zsh.



 Comments   
Comment by Dejan Josifovic [ 10/Apr/17 2:42 PM ]

Add patch.





[CLJS-2103]  clarify arg type and order constraints of keys and vals Created: 19/Jun/17  Updated: 19/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.562
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File CLJS-2103.patch    
Patch: Code

 Description   

Backport CLJ-1302 to ClojureScript, while also making the argument name be map instead of hash-map.






[CLJS-2120] Optimize keyword function Created: 24/Jun/17  Updated: 25/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.562
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-2120.patch    
Patch: Code
Approval: Accepted

 Description   

keyword function can be sped up. This matters when keywords are created dynamically when doing XHR.

The patch now matches Clojure more closely (using substring). It's also optimized for passing strings.

Results:

(enable-console-print!)
(let [sims 1000000]
  (dotimes [_ 2]
    (doseq [x ["foo" "foo/bar" [nil "foo"] ["foo" "bar"] [:foo :bar] [nil :foo]]]
      (prn "Testing keyword with: " x)
      (if (vector? x)
        (do (simple-benchmark [[a0 a1] x] (set! js/FOOO (keyword a0 a1)) sims)
            (simple-benchmark [[a0 a1] x] (set! js/FOOO (keyword2 a0 a1)) sims))
        (do (simple-benchmark [] (set! js/FOOO (keyword x)) sims)
            (simple-benchmark [] (set! js/FOOO (keyword2 x)) sims))))))


"Testing keyword with: " "foo"
[], (set! js/FOOO (keyword x)), 1000000 runs, 194 msecs
[], (set! js/FOOO (keyword2 x)), 1000000 runs, 71 msecs
"Testing keyword with: " "foo/bar"
[], (set! js/FOOO (keyword x)), 1000000 runs, 260 msecs
[], (set! js/FOOO (keyword2 x)), 1000000 runs, 104 msecs
"Testing keyword with: " [nil "foo"]
[[a0 a1] x], (set! js/FOOO (keyword a0 a1)), 1000000 runs, 278 msecs
[[a0 a1] x], (set! js/FOOO (keyword2 a0 a1)), 1000000 runs, 188 msecs
"Testing keyword with: " ["foo" "bar"]
[[a0 a1] x], (set! js/FOOO (keyword a0 a1)), 1000000 runs, 379 msecs
[[a0 a1] x], (set! js/FOOO (keyword2 a0 a1)), 1000000 runs, 215 msecs
"Testing keyword with: " [:foo :bar]
[[a0 a1] x], (set! js/FOOO (keyword a0 a1)), 1000000 runs, 351 msecs
[[a0 a1] x], (set! js/FOOO (keyword2 a0 a1)), 1000000 runs, 207 msecs
"Testing keyword with: " [nil :foo]
[[a0 a1] x], (set! js/FOOO (keyword a0 a1)), 1000000 runs, 376 msecs
[[a0 a1] x], (set! js/FOOO (keyword2 a0 a1)), 1000000 runs, 37 msecs


 Comments   
Comment by A. R [ 24/Jun/17 10:56 AM ]

Changes the behavior of:

((juxt namespace name) (keyword "foo/bar/hmm"))
=> [nil "foo"]
(.-fqn (keyword "foo/bar/hmm"))
=> "foo/bar/hmm"
((juxt namespace name) (keyword2 "foo/bar/hmm"))
=> ["foo" "bar/hmm"]
(.-fqn (keyword2 "foo/bar/hmm"))
=> "foo/bar/hmm"

Clojure 1.9:

((juxt namespace name) (keyword "foo/bar/hmm"))
=> ["foo" "bar/hmm"]

So: yay





[CLJS-2045] sort-by: Avoid re-creating comparator Created: 20/May/17  Updated: 25/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2045.patch    
Patch: Code

 Description   

The fn->comparator call should be lifted:

(sort (fn [x y] ((fn->comparator comp) (keyfn x) (keyfn y))) coll)
(let [comparator (fn->comparator comp)]
  (sort (fn [x y] (comparator (keyfn x) (keyfn y))) coll))

Also, fn->comparator is again called on the function in sort, not sure how to avoid that unless we copy the sort code into sort-by.






[CLJS-2038] self calls do not optimize - regression Created: 15/May/17  Updated: 25/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.293
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This is a regression of:

https://dev.clojure.org/jira/browse/CLJS-275



 Comments   
Comment by A. R [ 18/May/17 2:19 AM ]

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.

Comment by A. R [ 17/Jun/17 10:07 AM ]

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?

Comment by A. R [ 25/Jun/17 2:27 PM ]

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.





[CLJS-2132] Optimize transient vector creation Created: 27/Jun/17  Updated: 27/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.655
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-2132.patch    
Patch: Code

 Description   

This is a very simple optimization around transient []. It avoids copying the empty array.

Performance improvements, for mapv on smallish vectors (5-32) elements anywhere from 20% up to 100% across FF & Chrome.

(defn faster-editable-root
  [node]
  (if (identical? (.-EMPTY_NODE PersistentVector) node)
    (VectorNode. (js-obj) (make-array 32))
    (VectorNode. (js-obj) (aclone (.-arr node)))))
(def orig-editabe-root tv-editable-root)
(enable-console-print!)
(dotimes [_ 2]
  (doseq [size [5 10 40]]
    (let [xs (range size)
          sims 500000]
      (set! tv-editable-root orig-editabe-root)
      (prn "Size: " size)
      (simple-benchmark [] (mapv inc xs) sims)
      (set! tv-editable-root faster-editable-root)
      (prn "NEW:")
      (simple-benchmark [] (mapv inc xs) sims))))





[CLJS-1415] Handling JSDoc param name [x] optional syntax Created: 10/Aug/15  Updated: 27/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: type-check





[CLJS-994] print a warning when :externs file paths can't be found. Created: 30/Jan/15  Updated: 27/Jun/17

Status: Reopened
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Enhancement Priority: Minor
Reporter: Crispin Wellington Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: cljs, enhancement, errormsgs, patch,
Environment:

Linux 64bit

java version "1.7.0_65"
OpenJDK Runtime Environment (IcedTea 2.5.3) (7u71-2.5.3-0ubuntu0.14.04.1)
OpenJDK 64-Bit Server VM (build 24.65-b04, mixed mode)


Attachments: Text File clojurescript-extern-missing-warning.patch    
Patch: Code

 Description   

clojurescript silently ignores missing externs files possibly leading a developer to chase their tail.

Presently it can be very confusing using advanced compilation if you have made a mistake in the path name of one of your :externs files. This patch makes the compiler print a warning on stderr so you can quickly determine the cause of the broken advanced compilation output.

As a side effect, when doing a basic lein-cljsbuild a warning is always printed:

```
WARNING: js resource path closure-js/externs does not exist
```

This is because lein-cljsbuild quietly adds this extra path to your :externs listing without you knowing.



 Comments   
Comment by David Nolen [ 31/Jan/15 1:59 PM ]

You need to bind *out* to *err*, or just print to it directly a la cljs.util/debug-prn.

Comment by Crispin Wellington [ 31/Jan/15 7:30 PM ]

I did bind out to err. Check the patch.

Comment by David Nolen [ 01/Feb/15 12:30 PM ]

Crispin, oops sorry you are correct. Thanks.

Comment by David Nolen [ 13/Mar/15 7:33 AM ]

fixed https://github.com/clojure/clojurescript/commit/5f66a78bf469a9875e51aa39c29d3e66ce890eb4

Comment by David Nolen [ 14/Mar/15 5:55 AM ]

The solution does not work for cljsbuild. It's unclear why there so much machinery in place over the approach taken for deps.clj.

Comment by David Nolen [ 15/Mar/15 10:37 AM ]

Stalled on this cljsbuild issue https://github.com/emezeske/lein-cljsbuild/issues/383

Comment by Crispin Wellington [ 23/Mar/15 2:50 AM ]

This lein-cljsbuild issue is what made me make it just a warning initially, and not a hard error like raising IllegalArgumentException does. Though I agree it should be a hard error. If we start with a warning, it enables the immediate problem for the developer to be resolved, and leaves a wart that the cljs-build project can then see that need fixing on their end. Then when that end is fixed it could be made a hard error. If cljsbuild is fixed fairly soon then all is well, but if it takes a long time, a warning might be a good first step.





[CLJS-2147] apply test suit Created: 01/Jul/17  Updated: 01/Jul/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.671
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: test

Attachments: Text File CLJS-2147.patch    
Patch: Code and Test

 Description   

Created an apply test suit.

Few tests are commented out which currently fail (nothing new and there are tickets for them).



 Comments   
Comment by David Nolen [ 01/Jul/17 1:56 PM ]

Please remove #_ following each test.

Comment by A. R [ 01/Jul/17 2:14 PM ]

Forgot about those. Done.





[CLJS-2127] Add invoke* helper macro Created: 26/Jun/17  Updated: 03/Jul/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.562
Fix Version/s: 1.9.671

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: refactoring

Attachments: Text File CLJS-2127.patch    
Patch: Code
Approval: Screened

 Description   

This is a simple refactor around {IFn} protocol around core.cljc and core.cljs. We would like to hide the details of the invocation naming convention to avoid simple errors as well as to support changes more simply.



 Comments   
Comment by David Nolen [ 29/Jun/17 7:05 AM ]

The scope of this ticket needs to be narrowed to make it simpler for me to review. For the time being the only thing I would like to see is `invoke*` which hides the naming convention for direct invokes. No other higher level macro helpers should be provided in the resolution of this sissue.

Comment by A. R [ 29/Jun/17 12:29 PM ]

Patch updated. Much fewer changes to keep it simple for now.

Comment by David Nolen [ 29/Jun/17 2:08 PM ]

Looking better but lets have one helper for constructing the name, should just take number or :variadic.





[CLJS-2196] SpiderMonkey path needs quoting in test scripts Created: 08/Jul/17  Updated: 08/Jul/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.671
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2196.patch    
Patch: Code

 Description   

The paths to the other engines (V8, Nashorn, etc.), are quoted, allowing paths with spaces. This needs to also be done for SpiderMonkey.






[CLJS-2301] Avoid use of deprecated goog.string/isEmptySafe in clojure.string/blank? Created: 05/Aug/17  Updated: 05/Aug/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.854
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2301.patch    
Patch: Code

 Description   

clojure.string/blank? calls goog.string/isEmptySafe, which is marked as deprecated. Instead this can be inlined with the code that this internally calls, which is non-deprecated code. Also, it can be seen that such a change has no effect on perf, with these benchmarking tests tried out:

Before:

Benchmarking with V8
[s nil f clojure.string/blank?], (f s), 1000000 runs, 27 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 28 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 26 msecs
Benchmarking with SpiderMonkey
[s nil f clojure.string/blank?], (f s), 1000000 runs, 265 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 268 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 268 msecs
Benchmarking with JavaScriptCore
[s nil f clojure.string/blank?], (f s), 1000000 runs, 4 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 4 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 5 msecs
Benchmarking with Nashorn
[s nil f clojure.string/blank?], (f s), 1000000 runs, 331 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 336 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 59 msecs
Benchmarking with ChakraCore
[s nil f clojure.string/blank?], (f s), 1000000 runs, 56 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 56 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 57 msecs


After:

Benchmarking with V8
[s nil f clojure.string/blank?], (f s), 1000000 runs, 27 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 26 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 27 msecs
Benchmarking with SpiderMonkey
[s nil f clojure.string/blank?], (f s), 1000000 runs, 262 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 262 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 261 msecs
Benchmarking with JavaScriptCore
[s nil f clojure.string/blank?], (f s), 1000000 runs, 5 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 4 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 5 msecs
Benchmarking with Nashorn
[s nil f clojure.string/blank?], (f s), 1000000 runs, 328 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 324 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 60 msecs
Benchmarking with ChakraCore
[s nil f clojure.string/blank?], (f s), 1000000 runs, 62 msecs
[s " \n " f clojure.string/blank?], (f s), 1000000 runs, 63 msecs
[s "aBcDeF" f clojure.string/blank?], (f s), 1000000 runs, 62 msecs
Lindas-iMac-2:clojurescript mfikes$ # Using or nil?


 Comments   
Comment by Mike Fikes [ 05/Aug/17 11:37 AM ]

The change in the attached patch was used to produce the benchmarks in the ticket description.

I also tried an alternative which uses a Clojure or and a nil? check in lieu of the goog.string/makeSafe call, and this resulted in the same benchmark numbers.

So, the patch goes with the recommended deprecation change in the documentation for goog.string/isEmptyOrWhitespaceSafe, which indicates "Use goog.string.isEmptyOrWhitespace(goog.string.makeSafe(str)) instead.", which exactly matches the code we are indirectly calling today.





[CLJS-1997] Outward function type hint propagation Created: 03/Apr/17  Updated: 16/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJS-1997.patch    

 Description   

New feature: Detect the type of a function and arrange so that it is available for outward type hinting.

For reference, a small amount of discussion from Jan 2016 in the #cljs-dev Slack:

mfikes: Curious if "outward" ^boolean propagation has been debated / rejected. Example: If simple analysis detects that ^boolean can be applied as in for functions like (defn f [x] (not x)). Perhaps automatically propagating type hints in this direction would be unwelcome by users?

dnolen: @mfikes: we have already have outward propagation for numerics which broke because of changes for code motion

dnolen: absolutely nothing against that



 Comments   
Comment by Mike Fikes [ 03/Apr/17 7:38 PM ]

The attached patch will handle cases like

(defn foo? [x] (or (string? x) (number? x)))

and

(def baz (fn ([x] 1) ([x y] 2)))

but it doesn't attempt to address the case of multi-arity defn s, as these are implemented by dispatching to multiple single-arity functions that could have different return types.

It adds a :inferred-ret-tag to the AST only because the arguments to or on line 18 in the patch are not reversed (only to allow explicit hints to override inferred hints, but perhaps that is overcomplicating things and the inferred tag could be dropped directly into :ret-tag.)

Bronsa helpfully pointed out in Slack that outward type hint propagation is avoided in Clojure because it breaks things like:

(defn foo [] 1)
(defn bar [] (String/valueOf (foo)))
(defn foo [] "foo")
(bar)

for which an analogy in ClojureScript would be:

(defn foo? [] true)
(defn bar [] (if (foo?) :t :f))
(defn foo? [] "")
(bar)

where the patch causes it to yield :f.

This could be viewed as a REPL-only concern (perhaps addressed by another compiler flag along the lines of :static-fns).

But, arguably the problem already exists today, with type hinting in code like the following affecting the compilation of bar:

(def foo? true)
(defn bar [] (if foo? :t :f))
(def foo? "")
(bar)
Comment by David Nolen [ 07/Apr/17 11:15 AM ]

Thanks will think about it

Comment by Mike Fikes [ 16/Jun/17 10:30 AM ]

I've been soaking this for a couple of months (with local builds of Planck), and haven't seen anything break, FWIW. So, I think it is at least a safe change. Assigning to David for further consideration.





[CLJS-2341] Speed up js->clj on objs using forEach and transient volatile Created: 30/Aug/17  Updated: 31/Aug/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.908
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-2341.patch    
Patch: Code

 Description   

It is possible to speed up js->clj on JavaScript objects by revising the implementation to employ goog.object/forEach, accumulating by bashing on a transient volatile map.

The speedups range from 1.5 to 2.1 over the current implementation.

Note: The current implementation uses js-keys. The optimization in CLJS-2340 could help js->clj, but it doesn't appear to provide much speedup in itself (perhaps 1.1?) compared to the change in implementation described above.



 Comments   
Comment by Mike Fikes [ 30/Aug/17 9:19 AM ]
Speedup Summary:

        Engine  small, medium, with sub-object
            V8: 1.62, 1.50, 1.13
  SpiderMonkey: 1.91, 1.74, 1.59
JavaScriptCore: 1.67, 1.74, 2.10
       Nashorn: 1.54, 2.13, 1.51
    ChakraCore: 1.71, 2.10, 1.95


Before:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 55 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 63 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 93 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 155 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 87 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 94 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 45 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 33 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 42 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 1123 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 1195 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 773 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 65 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 44 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 78 msecs


After:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 34 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 42 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 82 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 81 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 50 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 59 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 27 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 19 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 20 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 728 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 561 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 513 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (js->clj obj), 1000 runs, 38 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (js->clj obj), 1000 runs, 21 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (js->clj obj), 1000 runs, 40 msecs




[CLJS-2346] Make (:require foo/bar) work for JS modules Created: 01/Sep/17  Updated: 05/Sep/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: js-modules

Attachments: Text File CLJS-2346.patch    
Patch: Code

 Description   

the string require only really becomes necessary when there are multiple slashes. We can support this style as a symbol for dependencies that only have 1 slash because it's a valid symbol.



 Comments   
Comment by Thomas Heller [ 02/Sep/17 2:43 AM ]

Me again ... what is so bad about using string requires? Qualified symbols are illegal for normal CLJS code and using a string already solves all potential issues. Adding a case where you can use a symbol if there is one slash but not if there are two is just going to confuse new users with no additional benefit.

Comment by Juho Teperi [ 05/Sep/17 3:40 AM ]

I can see the point of only allowing slashes with strings.

However, if we do this, it might be good to check the warnings we give when trying to do this:

(ns example.core
  (:require [react-dom/server :as sdf]))

  No such namespace: react-dom/server, could not locate react_dom_SLASH_server.cljs, react_dom_SLASH_server.cljc, or JavaScript source providing "server"

Notice how it says "server" instead of react-dom/server. This is because everything allows slashes, but foreign lib code only uses name part instead of namespace. (This patch doesn't change the warning.)

Comment by Thomas Heller [ 05/Sep/17 4:50 AM ]

There are other ambiguities when it comes to JS requires which is why I'm still advocating for just using strings in all circumstances.

Technically we could make {{(:require [@scoped/thing :as thing])}} work as well but shouldn't.

Also I'm pretty sure I saw several JS packages that either used "-" or "_" in their names. For CLJS we always convert to an underscore which would add more confusion if we don't do this for JS.

Yes, the warning should be fixed but wouldn't even be an issue if a string was used.





[CLJS-2342] Speed up printing of js object by using forEach and regex optimizations Created: 30/Aug/17  Updated: 01/Oct/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.908
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-2342-2.patch     Text File CLJS-2342.patch    
Patch: Code

 Description   

By using goog.object/forEach, along with direct interop for regex checking it is possible to speed up the printing of JavaScript objects anywhere between 1.14 and 1.77



 Comments   
Comment by Mike Fikes [ 30/Aug/17 10:50 AM ]
Speedup Summary:

        Engine  small, medium, with sub-object
            V8: 1.42, 1.31, 1.19
  SpiderMonkey: 1.14, 1.48, 1.41
JavaScriptCore: 1.48, 1.58, 1.62
       Nashorn: 1.27, 1.36, 1.20
    ChakraCore: 1.49, 1.77, 1.77


Before:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 74 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 84 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 75 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 95 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 92 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 134 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 46 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 41 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 55 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 1228 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 1048 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 620 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 55 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 76 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 129 msecs


After:

Benchmarking with V8
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 52 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 64 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 63 msecs

Benchmarking with SpiderMonkey
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 83 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 62 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 95 msecs

Benchmarking with JavaScriptCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 31 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 26 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 34 msecs

Benchmarking with Nashorn
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 970 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 769 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 518 msecs

Benchmarking with ChakraCore
[obj (js-obj "a" 1 "b" 2)], (pr-str obj), 1000 runs, 37 msecs
[obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6)], (pr-str obj), 1000 runs, 43 msecs
[sub-obj (js-obj "g" 7 "h" 8 "i" 9 "j" 10) obj (js-obj "a" 1 "b" 2 "c" 3 "d" 4 "e" 5 "f" 6 "s" sub-obj)], (pr-str obj), 1000 runs, 73 msecs
Comment by Mike Fikes [ 01/Oct/17 9:27 AM ]

Added CLJS-2342-2.patch which rebases against current master.





[CLJS-2300] Delegate clojure.string/capitalize to goog.string/capitalize Created: 04/Aug/17  Updated: 01/Oct/17

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.854
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File CLJS-2300-2.patch     Text File CLJS-2300-3.patch     Text File CLJS-2300.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Since the implementation of clojure.string/capitalize (6 years ago), Closure library added an identical function (3 years ago), which is actually much faster:

cljs.user=> (simple-benchmark [s "abc"] (goog.string/capitalize s) 10000000)
[s "abc"], (goog.string/capitalize s), 10000000 runs, 1181 msecs
nil
cljs.user=>  (simple-benchmark [s "abc"] (clojure.string/capitalize s) 10000000)
[s "abc"], (clojure.string/capitalize s), 10000000 runs, 5295 msecs

We could just have ClojureScript's delegate to Closure's.



 Comments   
Comment by Mike Fikes [ 04/Aug/17 9:17 PM ]

Before/After tests look promising:

Before:

Benchmarking with V8
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 166 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 380 msecs
Benchmarking with SpiderMonkey
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 833 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 1541 msecs
Benchmarking with JavaScriptCore
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 53 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 438 msecs
Benchmarking with Nashorn
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 903 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 1181 msecs
Benchmarking with ChakraCore
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 345 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 727 msecs

After:

Benchmarking with V8
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 28 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 28 msecs
Benchmarking with SpiderMonkey
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 263 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 263 msecs
Benchmarking with JavaScriptCore
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 5 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 5 msecs
Benchmarking with Nashorn
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 327 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 327 msecs
Benchmarking with ChakraCore
[s "a" f clojure.string/capitalize], (f s), 1000000 runs, 57 msecs
[s "aBcDeF" f clojure.string/capitalize], (f s), 1000000 runs, 63 msecs
Comment by Mike Fikes [ 04/Aug/17 10:53 PM ]

The benchmarks produced in the previous comment are with included in the attached patch.

The patch also includes a generative regression test, using the previous implementation as a reference.

All engines pass the test fine if you crank up the number of values tested, except for ChakraCore. ChakraCore appears to be buggy, and I'll follow up upstream with a ticket for them. At its core, if you capitalize "ß" you should get back "SS". With the original ClojureScript implementation, you get back "ﮰ" on ChakraCore, and with the revision in the patch (essentially Closure Library) you get back "鯪", neither of which are correct. So, this patch doesn't introduce a regression on ChakraCore.

Comment by Mike Fikes [ 05/Aug/17 11:59 AM ]

ChakraCore bug: https://github.com/Microsoft/ChakraCore/issues/421

Comment by Mike Fikes [ 01/Oct/17 9:34 AM ]

CLJS-2300-2.patch rebases against current master.

Comment by Mike Fikes [ 01/Oct/17 11:50 AM ]

I took a look at ChakraCore's upper-case bug, and it is fairly non-trivial for them to fix. (The code relies on upper-casing strings in-place and thus can't handle things like ß -> SS .) Additionally, SpiderMonkey does the wrong thing mapping ß -> ß.

Given that engines are evidently prone to get this wrong, and we don't want to live with unit tests failing forever, attached CLJS-2300-3.patch essentially works around this by limiting the generative test to covering ASCII strings.





[CLJS-1601] Optimize cljs.core dump Created: 12/Mar/16  Updated: 05/Oct/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.228
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Nikita Beloglazov Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: bootstrap

Attachments: Text File CLJS-1601.patch     Text File CLJS-1601.patch    
Patch: Code

 Description   

When building cljs source that uses cljs.js namespace the final js file is quite huge: 6.4M. As described in wiki: https://github.com/clojure/clojurescript/wiki/Optional-Self-hosting it mostly consists of analysis cache of the cljs.core namespace. As a workaround, the wiki article suggests dumping cache to a separate file and load it at runtime instead of bundling in js binary. I think it is possible to have something in between that doesn't require additional efforts from a user and also optimizes the size of the js file. The idea that instead of dumping cache as raw clojure data-structure it is serialized to string. This way compiler won't compile cache into js (which adds a lot of code) and leave it a string. At runtime, this string will be parsed back to clojure using tools.reader.

Here is the proposal: https://gist.github.com/nbeloglazov/0bf163fb62fa4b61d446

Checking locally it reduces the size of js file from 6.4M to 2.7M which I think quite good. The downside is that now js has to do more work on runtime (parse huge string) when today it simply read js code and evaluates it. But I don't think if it's a big concern. If it is desired to keep all behavior a new option can be added for :dump-core compiler setting, something like :dump-core :string that enables string serialization of the cache.

Does it sound reasonable?



 Comments   
Comment by Nikita Beloglazov [ 27/Mar/16 8:54 PM ]

Attaching suggested fix. Analysis cache is serialized to string and read back to clojure datastructure when cljs.js is initialized.

Comment by David Nolen [ 28/Mar/16 6:39 AM ]

Please change the patch so this optional as you've suggested.

Comment by David Nolen [ 28/Mar/16 6:40 AM ]

Also have you submitted your Clojure CA yet?

Comment by Nikita Beloglazov [ 28/Mar/16 1:35 PM ]

Will do. Yes, I've submitted CA. I used my official name, Mikita Belahlazau there.

Comment by Nikita Beloglazov [ 29/Mar/16 12:16 AM ]

Updated patch that adds option to serialize core analysis cache as string. Possible values of :dump-core are :raw, :string, :none. Old true/false values supported for backward compatibility.

As for default, current patch uses :raw, but I think it makes more sense to use :string. Saving extra few mb of final js is quite good. I think most devs won't go deep into figuring out why js is big and just leave it as it is. Additional one-time parsing performance hit :string introduces acceptable: when :string is used, page loads in 1s while with :raw the time is ~800ms.

Comment by David Nolen [ 16/Jun/17 12:41 PM ]

I'm questioning whether this actual valuable? It seems to me if you're serious about code size you would just use Transit and them load the analysis asynchronously?

Comment by Nikita Beloglazov [ 17/Jun/17 2:39 AM ]

Yes, if size is critical then there are better ways to hand-tune the way of loading analysis. At the same time having 3m vs 6m file for local/simple development is a nice win. The only downside is speed, but I feel like big reduction in size is better than small speed penalty.





[CLJS-2384] Old Infinity and NaN literals are still used in tests Created: 17/Oct/17  Updated: 17/Oct/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: 1.9.908

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2384.patch    
Patch: Code and Test

 Description   

You can see when running tests:

WARNING: Use of undeclared Var cljs.core-test/Infinity at line 1178 src/test/cljs/cljs/core_test.cljs
WARNING: Use of undeclared Var cljs.core-test/-Infinity at line 1179 src/test/cljs/cljs/core_test.cljs
WARNING: Use of undeclared Var cljs.core-test/NaN at line 1180 src/test/cljs/cljs/core_test.cljs
WARNING: Use of undeclared Var cljs.core-test/Infinity at line 1181 src/test/cljs/cljs/core_test.cljs
WARNING: Use of undeclared Var cljs.core-test/-Infinity at line 1181 src/test/cljs/cljs/core_test.cljs
WARNING: Use of undeclared Var cljs.core-test/Infinity at line 1182 src/test/cljs/cljs/core_test.cljs
WARNING: Use of undeclared Var cljs.core-test/-Infinity at line 1182 src/test/cljs/cljs/core_test.cljs





[CLJS-2383] Improve perf of select-keys by using keyword-identical? Created: 17/Oct/17  Updated: 23/Oct/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.908
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: performance

Attachments: Text File 0001-CLJS-2383-Speed-up-select-keys.patch     Text File 0002-CLJS-2383-Speed-up-select-keys-no-transient.patch     Text File CLJS-2383.patch    
Patch: Code and Test

 Description   

select-keys uses not= to compare keywords. Instead, using keyword-identical? results in decent speedups (an average of 1.34 across benchmarks and engines). Note that using identical? and lookup-sentinel doesn't appear to improve perf.

Speedup Summary:

V8: 1.15, 1.08, 1.08
  SpiderMonkey: 1.71, 1.48, 1.67
JavaScriptCore: 1.33, 1.35, 1.25
       Nashorn: 1.16, 1.04, 0.97
    ChakraCore: 1.59, 1.66, 1.72

Before:

Benchmarking with V8
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 179 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 121 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 183 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 251 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 201 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 290 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 112 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 73 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 119 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 1277 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 524 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 635 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 463 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 268 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 414 msecs

After

Benchmarking with V8
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 155 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 112 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 169 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 146 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 135 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 173 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 84 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 54 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 95 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 1099 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 502 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 648 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 292 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 151 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 240 msecs


 Comments   
Comment by Erik Assum [ 17/Oct/17 2:37 PM ]

Just want to bring your attention to CLJ-1789, where reimplementing `select-keys` in terms of reduce gave a significant speedup.
Added a patch to show that way.

Comment by Mike Fikes [ 18/Oct/17 6:46 AM ]

Here are the perf numbers for Erik's patch:

V8: 0.81, 1.14, 1.30
  SpiderMonkey: 1.49, 1.31, 1.58
JavaScriptCore: 1.02, 0.99, 0.96
       Nashorn: 1.13, 1.17, 1.21
    ChakraCore: 1.27, 1.35, 1.28

After:

Benchmarking with V8
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 220 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 106 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 141 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 169 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 153 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 183 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 110 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 74 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 124 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 1128 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 447 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 524 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs, 366 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs, 199 msecs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs, 323 msecs
Comment by Erik Assum [ 18/Oct/17 6:56 AM ]

Uploaded a patch without transients as well.

Comment by Mike Fikes [ 18/Oct/17 7:40 AM ]

Since the CLJ-1789 patch works better with larger maps, here is an additional perf test covering that case using the data from that ticket, testing both the original patch I had attached and Erik's subsequent patch. You can see the CLJ-1789 approach pays off for ClojureScript as well.

Erik, I see you attached a 3rd patch. I'd recommend adding perf numbers with each such patch, so the effect of the patch under advanced optimizations can be more readily assessed.

Engine          keyword-identical?  CLJ-1789
            V8:               1.13      1.29
  SpiderMonkey:               1.89      2.39
JavaScriptCore:               1.02      0.96
       Nashorn:               1.12      1.42
    ChakraCore:               1.68      1.82

Before:

Benchmarking with V8
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 373 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 668 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 200 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 2236 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 1074 msecs

After (keyword-identical?)

Benchmarking with V8
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 330 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 353 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 197 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 1991 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 640 msecs

After (CLJ-1789)

Benchmarking with V8
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 290 msecs

Benchmarking with SpiderMonkey
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 279 msecs

Benchmarking with JavaScriptCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 209 msecs

Benchmarking with Nashorn
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 1578 msecs

Benchmarking with ChakraCore
;;; select-keys
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs, 591 msecs
Comment by Erik Assum [ 18/Oct/17 7:54 AM ]

Both patches should have benchmarks now

Comment by Erik Assum [ 18/Oct/17 7:56 AM ]

oh, and as a comment to the comment about larger maps, I believe the performance `transient` bit is dependent on the size of the selected keys,
eg the more selected keys found in the map, the more we gain from `conj!`

Comment by Mike Fikes [ 23/Oct/17 12:04 PM ]

Running these 4 benchmarks

[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c]), 200000 runs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :x]), 200000 runs
[m {:a 1, :b 2, :c 3, :d 4}], (select-keys m [:a :b :c :x :y :z]), 200000 runs
[m {:a "b", :c "d", :b "b", :d "d", :e "e", :f "f", :g "g"}], (select-keys m [:a :c :b :d :e :f :g]), 200000 runs

against all 5 engines with the 3 attached patches yields the following speedups on my machine:

CLJS-2383.patch:

            V8: 1.11, 1.10, 1.64, 1.18  Avg: 1.26
  SpiderMonkey: 2.36, 1.46, 1.79, 2.02  Avg: 1.91
JavaScriptCore: 1.28, 1.34, 1.23, 1.49  Avg: 1.34
       Nashorn: 1.19, 1.17, 1.06, 1.29  Avg: 1.18
    ChakraCore: 1.61, 1.78, 1.75, 2.11  Avg: 1.81
                                Overall avg: 1.50
         Avg excluding Nashorn & ChakraCore: 1.50

0001-CLJS-2383-Speed-up-select-keys.patch:

            V8: 0.70, 0.95, 1.05, 1.23  Avg: 0.98
  SpiderMonkey: 1.20, 1.09, 1.05, 2.03  Avg: 1.34
JavaScriptCore: 0.78, 0.84, 0.83, 1.02  Avg: 0.87
       Nashorn: 1.18, 1.08, 1.02, 1.48  Avg: 1.19
    ChakraCore: 1.00, 1.12, 1.19, 1.75  Avg: 1.27
                                Overall avg: 1.13
         Avg excluding Nashorn & ChakraCore: 1.06	

0002-CLJS-2383-Speed-up-select-keys-no-transient.patch:
	
            V8: 1.28, 1.45, 1.37, 1.33  Avg: 1.36
  SpiderMonkey: 1.54, 1.44, 1.70, 2.17  Avg: 1.71
JavaScriptCore: 1.01, 0.99, 1.03, 1.13  Avg: 1.04
       Nashorn: 1.39, 1.21, 1.14, 1.26  Avg: 1.25
    ChakraCore: 1.20, 1.23, 1.19, 1.39  Avg: 1.25
                                Overall avg: 1.32
         Avg excluding Nashorn & ChakraCore: 1.37




[CLJS-1969] Docstring for condp indicates IllegalArgumentException Created: 07/Mar/17  Updated: 07/Mar/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File CLJS-1969.patch    
Patch: Code

 Description   

The docstring for condp indicates IllegalArgumentException is thrown. Instead it should indicate Error (as does case's docstring, for example.)






[CLJS-1974] Remove cljs.core/fixture1 and fixture2 Created: 11/Mar/17  Updated: 11/Mar/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-1974.patch    
Patch: Code

 Description   

There are a couple of vars in cljs.core named fixture1 and fixture2 that appear to perhaps be leftovers from when tests were assertions included in the cljs.core namespace.

https://github.com/clojure/clojurescript/commit/43861f7bc0b7b4fb534167b0773e49ccb68e6b1e#diff-a98a037c6c098dd3707e861df3c2f5acR1893

These can probably be safely removed.






[CLJS-2096] Avoid re-declaring deref Created: 16/Jun/17  Updated: 25/Jun/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.562
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: A. R Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2096.patch    
Patch: Code

 Description   

Currently every single @ or deref creates code like this:

$cljs$core$deref$$.$cljs$core$IFn$_invoke$arity$1$ ? $cljs$core$deref$$.$cljs$core$IFn$_invoke$arity$1$($G__11312_arr$jscomp$92_init__$2$$) : $cljs$core$deref$$.call(null, $G__11312_arr$jscomp$92_init__$2$$);

because it is declared after already defined. See CLJS-1992 .






[CLJS-2146] docstring for associative? should refer to IAssociative Created: 01/Jul/17  Updated: 01/Jul/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.671
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File CLJS-2146.patch    
Patch: Code

 Description   

The docstring currently mentions Associative:

cljs.user=> (doc associative?)
-------------------------
cljs.core/associative?
([x])
  Returns true if coll implements Associative
nil





[CLJS-2379] Print negative zero as -0.0 Created: 06/Oct/17  Updated: 06/Oct/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.908
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: enhancement, print

Attachments: Text File CLJS-2379.patch    
Patch: Code and Test

 Description   

In Clojure, negative zero is printed as -0.0 (for example when evaluating -0.0).

This ticket asks for the same for ClojureScript.

Note: The question may arise: Why not print as -0? The ClojureScript reader behaves similarly to Clojure's in that -0 is read in as an integer value, which doesn't have signed zeros, and is thus read in as 0. To properly round trip, this ticket asks that negative zero be printed as -0.0 (presuming that there are to be no changes to the reader behavior.)



 Comments   
Comment by Mike Fikes [ 06/Oct/17 8:24 AM ]

Oh hey Erik, I had assigned this ticket to myself and have attached a patch. I see you attached an alternative. But, I think your alternative prints 0 as -0. Is that correct?

Comment by Erik Assum [ 06/Oct/17 8:27 AM ]

Yeah, I didn't see you assigned to yourself, so I made a patch solving the description as it was (when it specified -0.

Comment by Mike Fikes [ 06/Oct/17 9:05 AM ]

The attached patch doesn't attempt to make (str -0.0) match Clojure (presuming that changing str could pose a perf challenge). But, it does give us round tripping for -0.0.

Here are perf benchmarks:

Speedup summary:

                 true     10  ##NaN  ##Inf ##-Inf (js-obj)  -0.0      0
            V8:  1.02   0.97   1.00   1.02   1.04    1.01   1.00   1.00
  SpiderMonkey:  0.98   1.04   1.00   1.01   0.98    1.02   1.05   1.03
JavaScriptCore:  1.00   1.01   1.02   1.01   1.01    1.04   1.04   1.00
       Nashorn:  0.98   1.13   0.96   0.95   0.99    0.92   0.99   0.98 
    ChakraCore:  1.02   1.03   1.03   1.02   1.01    0.99   1.07   1.07

Before:

Before:

Benchmarking with V8
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 386 msecs
[x 10], (pr-str x), 1000000 runs, 561 msecs
[x js/NaN], (pr-str x), 1000000 runs, 641 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 634 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 593 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 898 msecs
[x -0.0], (pr-str x), 1000000 runs, 449 msecs
[x 0], (pr-str x), 1000000 runs, 530 msecs

Benchmarking with SpiderMonkey
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 1321 msecs
[x 10], (pr-str x), 1000000 runs, 1330 msecs
[x js/NaN], (pr-str x), 1000000 runs, 1293 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 1307 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 1297 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 2143 msecs
[x -0.0], (pr-str x), 1000000 runs, 1340 msecs
[x 0], (pr-str x), 1000000 runs, 1312 msecs

Benchmarking with JavaScriptCore
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 502 msecs
[x 10], (pr-str x), 1000000 runs, 510 msecs
[x js/NaN], (pr-str x), 1000000 runs, 652 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 698 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 714 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 1266 msecs
[x -0.0], (pr-str x), 1000000 runs, 788 msecs
[x 0], (pr-str x), 1000000 runs, 562 msecs

Benchmarking with Nashorn
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 1746 msecs
[x 10], (pr-str x), 1000000 runs, 1498 msecs
[x js/NaN], (pr-str x), 1000000 runs, 1256 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 1251 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 1302 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 2905 msecs
[x -0.0], (pr-str x), 1000000 runs, 1342 msecs
[x 0], (pr-str x), 1000000 runs, 1383 msecs

Benchmarking with ChakraCore
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 1254 msecs
[x 10], (pr-str x), 1000000 runs, 1190 msecs
[x js/NaN], (pr-str x), 1000000 runs, 1118 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 1119 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 1124 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 2420 msecs
[x -0.0], (pr-str x), 1000000 runs, 1183 msecs
[x 0], (pr-str x), 1000000 runs, 1192 msecs

After:

After:

Benchmarking with V8
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 377 msecs
[x 10], (pr-str x), 1000000 runs, 577 msecs
[x js/NaN], (pr-str x), 1000000 runs, 642 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 623 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 570 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 887 msecs
[x -0.0], (pr-str x), 1000000 runs, 448 msecs
[x 0], (pr-str x), 1000000 runs, 530 msecs

Benchmarking with SpiderMonkey
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 1354 msecs
[x 10], (pr-str x), 1000000 runs, 1282 msecs
[x js/NaN], (pr-str x), 1000000 runs, 1320 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 1298 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 1322 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 2093 msecs
[x -0.0], (pr-str x), 1000000 runs, 1280 msecs
[x 0], (pr-str x), 1000000 runs, 1277 msecs

Benchmarking with JavaScriptCore
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 504 msecs
[x 10], (pr-str x), 1000000 runs, 507 msecs
[x js/NaN], (pr-str x), 1000000 runs, 639 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 690 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 708 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 1216 msecs
[x -0.0], (pr-str x), 1000000 runs, 759 msecs
[x 0], (pr-str x), 1000000 runs, 563 msecs

Benchmarking with Nashorn
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 1778 msecs
[x 10], (pr-str x), 1000000 runs, 1323 msecs
[x js/NaN], (pr-str x), 1000000 runs, 1303 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 1311 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 1309 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 3168 msecs
[x -0.0], (pr-str x), 1000000 runs, 1360 msecs
[x 0], (pr-str x), 1000000 runs, 1405 msecs

Benchmarking with ChakraCore
;; printing of numbers and handling of ##Nan, ##Inf, ##-Inf, -0.0
[x true], (pr-str x), 1000000 runs, 1233 msecs
[x 10], (pr-str x), 1000000 runs, 1150 msecs
[x js/NaN], (pr-str x), 1000000 runs, 1084 msecs
[x js/Infinity], (pr-str x), 1000000 runs, 1094 msecs
[x js/-Infinity], (pr-str x), 1000000 runs, 1117 msecs
[x (js-obj)], (pr-str x), 1000000 runs, 2445 msecs
[x -0.0], (pr-str x), 1000000 runs, 1106 msecs
[x 0], (pr-str x), 1000000 runs, 1119 msecs




Generated at Mon Oct 23 21:23:53 CDT 2017 using JIRA 4.4#649-r158309.