[CLJ-1073] Make print-sequential interruptible Created: 21/Sep/12 Updated: 01/Mar/13 |
|
| Status: | Open |
| Project: | Clojure |
| Component/s: | None |
| Affects Version/s: | Release 1.4 |
| Fix Version/s: | None |
| Type: | Enhancement | Priority: | Minor |
| Reporter: | Colin Jones | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | patch | ||
| Attachments: |
|
| Patch: | Code |
| Approval: | Vetted |
| Description |
|
This allows print-sequential to be interrupted by Thread.interrupt(), rather than requiring clients to resort to Thread.stop(). This is especially helpful when printing very large sequences. See also clojure-dev discussion at https://groups.google.com/d/topic/clojure-dev/vs0RNUQXiYE/discussion |
| Comments |
| Comment by Andy Fingerhut [ 21/Sep/12 7:04 PM ] |
|
In a quickly hacked up performance test on Mac OS X 10.6.8 + Oracle/Apple JDK 1.6.0_35 which I can attach, I saw about a 9% to 10% slowdown for Colin's patch in printing large vectors. I have a modified patch that only calls Thread/interrupted after every 20 items are printed, and with that version the slowdown versus the original code was about 3% to 4%. Colin, would there be any down side to checking Thread/interrupted less often for your purposes? To run performance test, edit attachment compile.sh to point at your clojure.jar, put file perftest-print.clj in the same directory, and run ./compile.sh It should work on Mac or Linux. |
| Comment by Andy Fingerhut [ 22/Sep/12 10:47 AM ] |
|
clj-1073-allow-thread-interrupt-in-print-sequential-patch.txt dated Sep 22 2012 is similar to Colin's patch 0001-Allow-thread-interruption-in-print-sequential.patch dated Sep 21 2012, except it only checks interrupted status every 20 (or maybe 21?) times through the loop in print-sequential. It is the one that is 3-4% slower than the current latest master Clojure code in my performance test mentioned above, versus Colin's patch which is about 9-10% slower on that test. |
| Comment by Stuart Halloway [ 19/Oct/12 3:31 PM ] |
|
Is this primarily intended for dev-time use? I wouldn't want to lose performance for this if there is any way to implement it as a dev-time feature. |
| Comment by Colin Jones [ 19/Oct/12 4:27 PM ] |
|
Andy: The only caveat I can think of with checking Thread/interrupted less often is in the case of deeply nested collections. Stu: Dev-time use was the original reason for opening this, yes. But I can imagine it being needed for anytime a thread can be interrupted, whether that's by ctrl-c or other means. My original thinking, performance-wise, was that once we're already doing IO, we're probably not too concerned with CPU-bound checks like this, so I didn't bother with it. I guess with an SSD that's not as likely to be true. Locally (w/ my SSD), I'm seeing that Andy's benchmark of printing a million numbers is about a second slower than the baseline with my original patch (12.08s -> 13.10s), and about the same with Andy's patch (12.08s -> 11.75s). Decreasing to a thousand numbers doesn't really show any difference (every version completes in ~1.3s). Bumping to 2 million adds 2 seconds above the baseline with my patch, and Andy's is again just a bit faster than the baseline (somehow). Bumping to 5 million runs me out of heap space |
| Comment by Andy Fingerhut [ 08/Nov/12 4:16 PM ] |
|
clj-1073-add-print-interruptibly-patch-v1.txt dated Nov 8 2012 is the same idea as Colin's patch 0001-Allow-thread-interruption-in-print-sequential.patch dated Sep 21 2012, except it only checks (Thread/interrupted) if a new var print-interruptibly is true. Its default value is false. Performance results of the perftest-print.clj program, as driven by the test.sh script, for Clojure 1.5-beta1 and with two different patches. All run times are elapsed, in seconds, and sorted in increasing order for easier comparison. Executive summary: Performance results when print-interruptibly is left at default false value are pretty much same as today. With print-interruptibly bound to true, performance results are slower, as expected, and about the same as with Colin's patch. Original 1.5-beta1 unchanged: With this new print-interruptibly patch, with print-interruptibly With this new print-interruptibly patch, with print-interruptibly With patch 0001-Allow-thread-interruption-in-print-sequential.patch |
| Comment by Colin Jones [ 12/Nov/12 1:37 PM ] |
|
Andy's latest patch looks good & makes it easy for the REPL and other interruptible scenarios to opt into the "safe" behavior. I would personally have preferred to have the "safe" behavior on by default, but can understand the performance concerns, and this gets me what I really wanted. |
| Comment by Timothy Baldridge [ 30/Nov/12 3:09 PM ] |
|
Vetting as it sounds like the performance issues are taken care of. |
| Comment by Andy Fingerhut [ 13/Feb/13 12:28 AM ] |
|
clj-1073-add-print-interruptibly-patch-v2.txt dated Feb 12 2013 is identical to clj-1073-add-print-interruptibly-patch-v1.txt dated Nov 8 2012 (soon to be removed) except that it applies cleanly to latest master. |