<< Back to previous view

[CLJS-801] str macro emits unoptimizable js code Created: 27/Apr/14  Updated: 11/May/14  Resolved: 11/May/14

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

Type: Enhancement Priority: Minor
Reporter: Francis Avila Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: string


Attachments: Text File cljs-801.patch     Text File cljs-801-v2.patch     Text File cljs-801-v3.patch     Text File cljs-801-v4.patch    
Patch: Code and Test


Clojurescript's str macro emits javascript code which is inefficient and which the closure compiler cannot optimize.

Currently it emits code like {{[cljs.core.str(arg1),cljs.core.str(arg2)].join('')}}. The problems with this:

  1. The emitted function is the arity-dispatch str wrapper instead of the 1-arg implementation of str. The closure compiler cannot eliminate the dispatch.
  2. An intermediate array is always created; the closure compiler cannot optimize it out.
  3. The closure compiler can evaluate constant string expressions (e.g. 'a'+1 to 'a1'), but cannot in this case because it cannot eliminate the str call.

The attached patch rewrites the str macro to generate js code that looks like this:

(str arg1 "constant" \space true nil 123 456.78)

(""+cljs.core.str.cljs$core$IFn$_invoke$arity$1(arg1)+"constant "+true+123+456.78)

This has a number of benefits:

  1. No short-lived array or Array.join operation.
  2. No arity dispatch is invoked. I have also observed that it can (but won't necessarily) inline the function body.
  3. The compiler can perform constant evaluation. For example, in advanced mode the compiler will emit the above example as (""+w.c(a)+"constant true123456.78") where w.c is the munged cljs.core.str.cljs$core$IFn$_invoke$arity$1

Comment by Francis Avila [ 28/Apr/14 12:17 AM ]

Updated patch adds booleans to the str test case, and does not eagerly stringify bools anymore. (It emits as literals and lets the closure compiler decide to stringify at compile time if it wants.)

Comment by David Nolen [ 10/May/14 2:23 PM ]

Need the patch rebased to master.

Comment by Francis Avila [ 11/May/14 1:38 AM ]

Updated patch.

Comment by David Nolen [ 11/May/14 10:52 AM ]

Sorry because of the last commit to master this ticket will not apply, please rebase again. I'll refrain from adding tests until this one gets merged in

Comment by Francis Avila [ 11/May/14 11:22 AM ]

No problem, updated patch.

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

fixed https://github.com/clojure/clojurescript/commit/c63db9544738fc094cc04680bbee1534cf436204

Generated at Thu Oct 19 18:53:18 CDT 2017 using JIRA 4.4#649-r158309.