Clojure

slurp should not read one character at a time

Details

  • Type: Enhancement Enhancement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Declined
  • Affects Version/s: Approved Backlog
  • Fix Version/s: Release 1.4
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code
  • Approval:
    Incomplete

Description

A discussion of slurp performance came up on the mailing list. The attached patch makes slurp use a 4kb buffer size instead of reading one character at a time, resulting in a significant performance improvement.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Assembla Importer added a comment -

stu said: Updating tickets (#389, #371)

Show
Assembla Importer added a comment - stu said: Updating tickets (#389, #371)
Hide
Permalink
Assembla Importer added a comment -

aaron said: Patch does not apply. It is not in the proper format. Please take a look at http://clojure.org/patches and resubmit. I will continue to review the code itself against the current master.

Show
Assembla Importer added a comment - aaron said: Patch does not apply. It is not in the proper format. Please take a look at http://clojure.org/patches and resubmit. I will continue to review the code itself against the current master.
Hide
Permalink
Aaron Bedra added a comment -

Contacted original poster about this to inform him of the move to JIRA and ensure interest in this patch

Show
Aaron Bedra added a comment - Contacted original poster about this to inform him of the move to JIRA and ensure interest in this patch
Hide
Permalink
Aaron Bedra added a comment -

The patch has been reformatted and submitted.

Show
Aaron Bedra added a comment - The patch has been reformatted and submitted.
Hide
Permalink
Jürgen Hötzel added a comment -

Maybe duplicate: CLJ-668

Show
Jürgen Hötzel added a comment - Maybe duplicate: CLJ-668
Hide
Permalink
Aaron Bedra added a comment - - edited

CLJ-668 is a duplicate of this ticket. I have tested your patch and would like to consider it as a resolution to this ticket along with closing CLJ-668 as a duplicate. Can you please submit a patch to this ticket instead of a link to github? I am going to test all of the patches presented on OSX, Linux, EC2, and Windows.

Show
Aaron Bedra added a comment - - edited CLJ-668 is a duplicate of this ticket. I have tested your patch and would like to consider it as a resolution to this ticket along with closing CLJ-668 as a duplicate. Can you please submit a patch to this ticket instead of a link to github? I am going to test all of the patches presented on OSX, Linux, EC2, and Windows.
Hide
Permalink
Jürgen Hötzel added a comment - - edited

Seems like the changes are already applied by your previous commit:

https://github.com/clojure/clojure/commit/cbd789d1a5b472d92b91f2fe0e273f48c2583483

Just a leftover StringBuilder Object (patch attached)

Show
Jürgen Hötzel added a comment - - edited Seems like the changes are already applied by your previous commit: https://github.com/clojure/clojure/commit/cbd789d1a5b472d92b91f2fe0e273f48c2583483 Just a leftover StringBuilder Object (patch attached)
Hide
Permalink
Aaron Bedra added a comment -

Yes, that was a mistake. It should not have gone in under #441. I was doing some testing and somehow I didn't properly revert what I was working on. Please consider that a mistake for now and include a patch here so you can get credit for your work.

Show
Aaron Bedra added a comment - Yes, that was a mistake. It should not have gone in under #441. I was doing some testing and somehow I didn't properly revert what I was working on. Please consider that a mistake for now and include a patch here so you can get credit for your work.
Hide
Permalink
Jürgen Hötzel added a comment -

Thanks. Patch enclosed.

Show
Jürgen Hötzel added a comment - Thanks. Patch enclosed.
Hide
Permalink
Aaron Bedra added a comment -

Thanks. I am going to be testing the various ideas on multiple platforms to see which (if any) is the best fit.

Show
Aaron Bedra added a comment - Thanks. I am going to be testing the various ideas on multiple platforms to see which (if any) is the best fit.
Hide
Permalink
Aaron Bedra added a comment - - edited

This patch carries a performance hit. For example:

Clojure 1.4.0

File Size best timed 10x slurp
7.3 MB 2960.363 msecs
260 KB 105.255 msecs

With Patch

File Size best timed 10x slurp
7.3 MB 3700.406 msecs
260 KB 129.311 msecs

This was tested with

(time (dotimes [_ 10] (slurp "file")))

with each execution of this form done 20 times.

Show
Aaron Bedra added a comment - - edited This patch carries a performance hit. For example: Clojure 1.4.0
File Size best timed 10x slurp
7.3 MB 2960.363 msecs
260 KB 105.255 msecs
With Patch
File Size best timed 10x slurp
7.3 MB 3700.406 msecs
260 KB 129.311 msecs
This was tested with (time (dotimes [_ 10] (slurp "file"))) with each execution of this form done 20 times.

People

Vote (0)
Watch (2)

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.