Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix println performance regression #10679

Merged
merged 1 commit into from
Apr 2, 2015
Merged

fix println performance regression #10679

merged 1 commit into from
Apr 2, 2015

Conversation

amitmurthy
Copy link
Contributor

Addresses #10650

The first time is for writing to a file, the second to STDOUT

$ time julia i2050.jl
real    0m37.606s
user    0m35.112s
sys     0m1.615s

$ time julia i2050_redirect_stdout.jl > juliadump
real    0m39.467s
user    0m37.066s
sys     0m1.787s

The current patch

  • locks println only for IO types defining a lock field. Currently only AsyncStream types.
  • exports lock, unlock and ReentrantLock. Users writing concurrently to IOStream objects, may lock their writes accordingly. Usage is to be documented.

edit: changed to use stagedfunction
edit2: removed stagedfunction. Only println on AsyncStream is locked.

lock(io) do io
for x in xs print(io, x) end
stagedfunction print(io::IO, xs...)
if :lock in fieldnames(io)
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather this not be a staged function. It's overkill, since it will force recompilation for every combination of trailing arguments, whose types aren't even used. Is an isdefined check too slow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never realised that! isdefined adds around 1.5 - 2.0 seconds to the results.

@amitmurthy
Copy link
Contributor Author

Removed the stagedfunction. Only println on AsyncStream objects are locked.

The timings below are on a different system (a MacBook Pro running Ubuntu). The range of timings across multiple runs is quite large on this machine - don't have an explanation for that, but both non-AsyncStream println and AsyncStream println seem to be in the same ballpark.

4 runs of `time julia i2050.jl`

real    0m34.614s
user    0m33.841s
sys     0m1.663s

real    0m45.005s
user    0m41.723s
sys     0m2.425s

real    0m44.310s
user    0m40.538s
sys     0m2.378s

real    0m47.341s
user    0m43.903s
sys     0m2.502s


4 runs of `time julia i2050_redirect_stdout.jl > juliadump`
real    0m37.469s
user    0m36.639s
sys     0m1.678s

real    0m42.083s
user    0m41.228s
sys     0m1.730s

real    0m44.907s
user    0m43.908s
sys     0m1.886s

real    0m46.356s
user    0m45.088s
sys     0m2.153s

@amitmurthy amitmurthy force-pushed the amitm/lockperf branch 2 times, most recently from 6813f44 to 10030f4 Compare April 1, 2015 05:34
@amitmurthy
Copy link
Contributor Author

Added docs. Will squash and merge in a day or two if there aren't any concerns.

amitmurthy added a commit that referenced this pull request Apr 2, 2015
fix println performance regression
@amitmurthy amitmurthy merged commit 09ca2e1 into master Apr 2, 2015
@amitmurthy amitmurthy deleted the amitm/lockperf branch April 2, 2015 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants