Clojure

Make print-sequential interruptible

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.4, Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Patch:
    Code
  • Approval:
    Triaged

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

Patch: clj-1073-add-print-interruptibly-patch-v2.txt

Approach:

Add a new var print-interruptibly, similar to print-length, print-dup, etc. Its default is false. When true, (Thread/interrupted) is checked for after printing each element of a sequence. If true, the writer is flushed and an InterruptedIOException is thrown.

An alternative patch proposed earlier did not include print-interruptibly, but simply always checked (Thread/interrupted) after printing each element of a sequence. Benchmark tests showed that this could slow down printing of long sequences by about 10%. The approach in the proposed patch only incurs this slowdown if print-interruptibly is true. When false, there is no significant slowdown measured in benchmarks.

  1. 0001-Allow-thread-interruption-in-print-sequential.patch
    21/Sep/12 4:10 PM
    1 kB
    Colin Jones
  2. clj-1073-add-print-interruptibly-patch-v2.txt
    13/Feb/13 12:28 AM
    4 kB
    Andy Fingerhut
  3. perftest-print.clj
    21/Sep/12 7:05 PM
    0.3 kB
    Andy Fingerhut
  4. test.sh
    21/Sep/12 7:05 PM
    0.3 kB
    Andy Fingerhut

Activity

Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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.
Andy Fingerhut made changes -
Field Original Value New Value
Attachment test.sh [ 11518 ]
Andy Fingerhut made changes -
Attachment perftest-print.clj [ 11519 ]
Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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.
Andy Fingerhut made changes -
Attachment clj-1073-allow-thread-interrupt-in-print-sequential-patch.txt [ 11522 ]
Hide
Stuart Halloway added a comment -

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.

Show
Stuart Halloway added a comment - 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.
Hide
Colin Jones added a comment -

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

Show
Colin Jones added a comment - 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
Hide
Andy Fingerhut added a comment -

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:
13.75 13.80 13.83 13.87 13.93

With this new print-interruptibly patch, with print-interruptibly
at default false value:
13.86 13.91 14.01 14.08 14.14

With this new print-interruptibly patch, with print-interruptibly
bound to true while printing (so a slightly modified version of
perftest-print.clj that does (binding [*print-interruptibly* true]
...) around the heart of the code):
15.29 15.44 15.45 15.62 15.63

With patch 0001-Allow-thread-interruption-in-print-sequential.patch
applied:
15.38 15.46 15.48 15.49 15.50

Show
Andy Fingerhut added a comment - 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: 13.75 13.80 13.83 13.87 13.93 With this new print-interruptibly patch, with print-interruptibly at default false value: 13.86 13.91 14.01 14.08 14.14 With this new print-interruptibly patch, with print-interruptibly bound to true while printing (so a slightly modified version of perftest-print.clj that does (binding [*print-interruptibly* true] ...) around the heart of the code): 15.29 15.44 15.45 15.62 15.63 With patch 0001-Allow-thread-interruption-in-print-sequential.patch applied: 15.38 15.46 15.48 15.49 15.50
Andy Fingerhut made changes -
Attachment clj-1073-add-print-interruptibly-patch-v1.txt [ 11667 ]
Andy Fingerhut made changes -
Attachment clj-1073-allow-thread-interrupt-in-print-sequential-patch.txt [ 11522 ]
Hide
Colin Jones added a comment -

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.

Show
Colin Jones added a comment - 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.
Timothy Baldridge made changes -
Assignee Timothy Baldridge [ halgari ]
Hide
Timothy Baldridge added a comment -

Vetting as it sounds like the performance issues are taken care of.

Show
Timothy Baldridge added a comment - Vetting as it sounds like the performance issues are taken care of.
Timothy Baldridge made changes -
Approval Vetted [ 10003 ]
Assignee Timothy Baldridge [ halgari ]
Hide
Andy Fingerhut added a comment -

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.

Show
Andy Fingerhut added a comment - 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.
Andy Fingerhut made changes -
Andy Fingerhut made changes -
Attachment clj-1073-add-print-interruptibly-patch-v1.txt [ 11667 ]
Stuart Halloway made changes -
Issue Type Defect [ 1 ] Enhancement [ 4 ]
Hide
Alex Miller added a comment -

Moving back to Triaged as Rich has not vetted.

Show
Alex Miller added a comment - Moving back to Triaged as Rich has not vetted.
Alex Miller made changes -
Approval Vetted [ 10003 ] Triaged [ 10120 ]
Andy Fingerhut made changes -
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
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

*Patch:* clj-1073-add-print-interruptibly-patch-v2.txt

*Approach:*

Add a new var {{*print-interruptibly*}}, similar to {{*print-length*}}, {{*print-dup*}}, etc. Its default is false. When true, (Thread/interrupted) is checked for after printing each element of a sequence. If true, the writer is flushed and an InterruptedIOException is thrown.

An alternative patch proposed earlier did not include {{*print-interruptibly*}}, but simply always checked (Thread/interrupted) after printing each element of a sequence. Benchmark tests showed that this could slow down printing of long sequences by about 10%. The approach in the proposed patch only incurs this slowdown if {{*print-interruptibly*}} is true. When false, there is no significant slowdown measured in benchmarks.
Affects Version/s Release 1.5 [ 10150 ]
Labels patch
Andy Fingerhut made changes -
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

*Patch:* clj-1073-add-print-interruptibly-patch-v2.txt

*Approach:*

Add a new var {{*print-interruptibly*}}, similar to {{*print-length*}}, {{*print-dup*}}, etc. Its default is false. When true, (Thread/interrupted) is checked for after printing each element of a sequence. If true, the writer is flushed and an InterruptedIOException is thrown.

An alternative patch proposed earlier did not include {{*print-interruptibly*}}, but simply always checked (Thread/interrupted) after printing each element of a sequence. Benchmark tests showed that this could slow down printing of long sequences by about 10%. The approach in the proposed patch only incurs this slowdown if {{*print-interruptibly*}} is true. When false, there is no significant slowdown measured in benchmarks.
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

*Patch:* clj-1073-add-print-interruptibly-patch-v2.txt

*Approach:*

Add a new var *print-interruptibly*, similar to *print-length*, *print-dup*, etc. Its default is false. When true, (Thread/interrupted) is checked for after printing each element of a sequence. If true, the writer is flushed and an InterruptedIOException is thrown.

An alternative patch proposed earlier did not include *print-interruptibly*, but simply always checked (Thread/interrupted) after printing each element of a sequence. Benchmark tests showed that this could slow down printing of long sequences by about 10%. The approach in the proposed patch only incurs this slowdown if *print-interruptibly* is true. When false, there is no significant slowdown measured in benchmarks.
Alex Miller made changes -
Labels print

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated: