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

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

Activity

Hide
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
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
David Nolen added a comment -

on V8? JSC? SM?

Show
David Nolen added a comment - on V8? JSC? SM?
Hide
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
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
David Nolen made changes -
Field Original Value New Value
Priority Minor [ 4 ] Major [ 3 ]
Hide
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
Paul deGrandis made changes -
Assignee Paul deGrandis [ ohpauleez ]
Paul deGrandis made changes -
Attachment cljs-321-with-out-str.diff [ 11600 ]
Hide
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
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
Paul deGrandis added a comment -

No problem - on it right now.

Show
Paul deGrandis added a comment - No problem - on it right now.
Paul deGrandis made changes -
Attachment cljs-321-with-out-str.diff [ 11601 ]
Hide
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.
David Nolen made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Resolved [ 5 ]
David Nolen made changes -
Status Resolved [ 5 ] Closed [ 6 ]

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: