<< Back to previous view

[CLJ-389] slurp should not read one character at a time Created: 25/Jun/10  Updated: 26/Jul/13  Resolved: 02/Dec/11

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Backlog
Fix Version/s: Release 1.4

Type: Enhancement Priority: Minor
Reporter: Aaron Bedra Assignee: Aaron Bedra
Resolution: Declined Votes: 0
Labels: None

Attachments: Text File 0001-Improve-slurp-performance-by-using-native-Java-Strin.patch     Text File 0001-needles-StrinbBuilder-in-slurp.patch     File 0389_buffered_slurp.diff    
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.



 Comments   
Comment by Assembla Importer [ 25/Oct/10 9:30 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/389
Attachments:
slurp-buffer-size.patch - https://www.assembla.com/spaces/clojure/documents/c7INhmGfSr34YDeJe5cbCb/download/c7INhmGfSr34YDeJe5cbCb

Comment by Assembla Importer [ 25/Oct/10 9:30 PM ]

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

Comment by Assembla Importer [ 25/Oct/10 9:30 PM ]

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.

Comment by Aaron Bedra [ 29/Oct/10 9:43 AM ]

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

Comment by Aaron Bedra [ 12/Nov/10 1:55 PM ]

The patch has been reformatted and submitted.

Comment by Jürgen Hötzel [ 13/Nov/10 2:07 AM ]

Maybe duplicate: CLJ-668

Comment by Aaron Bedra [ 26/Nov/10 10:17 AM ]

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.

Comment by Jürgen Hötzel [ 27/Nov/10 4:57 AM ]

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)

Comment by Aaron Bedra [ 28/Nov/10 10:06 AM ]

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.

Comment by Jürgen Hötzel [ 29/Nov/10 2:50 AM ]

Thanks. Patch enclosed.

Comment by Aaron Bedra [ 03/Dec/10 8:19 AM ]

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

Comment by Aaron Bedra [ 30/Nov/11 7:59 PM ]

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.

Generated at Fri Aug 29 21:13:39 CDT 2014 using JIRA 4.4#649-r158309.