ClojureScript

Support with-out-str

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test

Description

Attached patch implements with-out-str macro in terms of goog.string.StringBuffer and print-fn via .append

The attached patch also fixes CLJS-319

Attachments

  1. Text File
    with-out-str.patch
    18/Jun/12 3:15 AM
    5 kB
    Brandon Bloom
  2. File
    cljs-321-with-out-str.diff
    22/Oct/12 4:55 PM
    0.9 kB
    Paul deGrandis
  3. File
    cljs-321-with-out-str.diff
    22/Oct/12 5:17 PM
    3 kB
    Paul deGrandis

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
David Nolen added a comment -

Patch looks mostly OK. Did you do any basic benchmarking? It would be nice to know that this doesn't slow down printing of Clojure objects.

Show
David Nolen added a comment - Patch looks mostly OK. Did you do any basic benchmarking? It would be nice to know that this doesn't slow down printing of Clojure objects.
Hide
Permalink
Brandon Bloom added a comment -

Yes, I ran this:

(time (dotimes [i 5000] (prn-str {:foo [1 "two" 'three]}))

before patch: 29988 msecs
after patch: 28751 msecs

Effectively identical.

Show
Brandon Bloom added a comment - Yes, I ran this: (time (dotimes [i 5000] (prn-str {:foo [1 "two" 'three]})) before patch: 29988 msecs after patch: 28751 msecs Effectively identical.
Hide
Permalink
David Nolen added a comment -

on V8? JSC? SM?

Show
David Nolen added a comment - on V8? JSC? SM?
Hide
Permalink
Brandon Bloom added a comment -

I had run it in a browser repl, but I forget which. I added a benchmark and re-ran it in all three. It was a tiny bit slower, so I went to investigate and discovered that I haven't the slightest clue how to predict or interpret Javascript engine performance numbers. Hell, Chrome's console seems to have completely random performance, where as the Node.js repl seems to be more consistent. I'll get my shit together and see if I can fix this up. Hopefully will have time to look at it tomorrow.

Show
Brandon Bloom added a comment - I had run it in a browser repl, but I forget which. I added a benchmark and re-ran it in all three. It was a tiny bit slower, so I went to investigate and discovered that I haven't the slightest clue how to predict or interpret Javascript engine performance numbers. Hell, Chrome's console seems to have completely random performance, where as the Node.js repl seems to be more consistent. I'll get my shit together and see if I can fix this up. Hopefully will have time to look at it tomorrow.
Hide
Permalink
Laszlo Török added a comment -

jsperf.com was a great help to me when prototyping PersistentVector's first CLJS implementation

it's very-very handy and easy to use and keep updated if necessary

e.g.
http://jsperf.com/persistentvector-norecur-js/12

Show
Laszlo Török added a comment - jsperf.com was a great help to me when prototyping PersistentVector's first CLJS implementation it's very-very handy and easy to use and keep updated if necessary e.g. http://jsperf.com/persistentvector-norecur-js/12
Hide
Permalink
David Nolen added a comment -

Thanks Brandon, let's hold off on this one until we can resolve CLJS-340

Show
David Nolen added a comment - Thanks Brandon, let's hold off on this one until we can resolve CLJS-340
Hide
Permalink
Paul deGrandis added a comment -

I attached an updated patch (that I'm using internally for a personal project) of just the with-out-str macro code.

Show
Paul deGrandis added a comment - I attached an updated patch (that I'm using internally for a personal project) of just the with-out-str macro code.
Hide
Permalink
David Nolen added a comment -

Thanks Paul. Could we get a patch with tests included?

Show
David Nolen added a comment - Thanks Paul. Could we get a patch with tests included?
Hide
Permalink
Paul deGrandis added a comment -

No problem - on it right now.

Show
Paul deGrandis added a comment - No problem - on it right now.
Hide
Permalink
Paul deGrandis added a comment -

Updated with one additional piece I needed to bring over from my branch (:dynamic on print-fn) and two tests: one using print and one using the raw print-fn.

Show
Paul deGrandis added a comment - Updated with one additional piece I needed to bring over from my branch (:dynamic on print-fn) and two tests: one using print and one using the raw print-fn.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.