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 a few spaceleaks #413

Merged
merged 1 commit into from Jan 13, 2017

Conversation

Projects
None yet
2 participants
@cocreature
Collaborator

cocreature commented Oct 8, 2016

This is the result of applying Neil Mitchell’s trick of limiting the stack size to hledger.

Let me break down the different changes:

  • sum, maximum and minimum use foldl instead of foldl' which can leak memory.
  • Since asprecision is rarely evaluated this is responsible for another spaceleak.
  • Lastly sequence is sort of a false positive so I mostly made the change here to be able to use the trick. Looking at benchmarks it seems like this difference list based version is slightly faster on small lists but is slower on bigger lists.

Here are the benchmark results:

before

Benchmark bench: RUNNING...
Benchmarking hledger in /home/moritz/code/haskell/hledger/baseline/hledger with criterion
benchmarking read bench/10000x1000x10.journal
time                 1.992 s    (1.990 s .. 1.995 s)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 1.987 s    (1.986 s .. 1.988 s)
std dev              1.392 ms   (0.0 s .. 1.543 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking print
time                 1.599 s    (1.583 s .. 1.607 s)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 1.612 s    (1.606 s .. 1.614 s)
std dev              5.457 ms   (0.0 s .. 6.127 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking register
time                 2.203 s    (2.161 s .. 2.261 s)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 2.159 s    (2.131 s .. 2.173 s)
std dev              24.04 ms   (0.0 s .. 24.94 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking balance
time                 209.6 ms   (204.6 ms .. 214.6 ms)
                     1.000 R²   (0.999 R² .. 1.000 R²)
mean                 208.9 ms   (208.1 ms .. 211.0 ms)
std dev              1.625 ms   (24.09 μs .. 2.165 ms)
variance introduced by outliers: 14% (moderately inflated)

benchmarking stats
time                 195.6 ms   (189.5 ms .. 201.6 ms)
                     0.999 R²   (0.996 R² .. 1.000 R²)
mean                 197.3 ms   (194.6 ms .. 201.5 ms)
std dev              4.471 ms   (2.152 ms .. 6.746 ms)
variance introduced by outliers: 14% (moderately inflated)

after

Benchmark bench: RUNNING...
Benchmarking hledger in /home/moritz/code/haskell/hledger/hledger with criterion
benchmarking read bench/10000x1000x10.journal
time                 1.852 s    (1.817 s .. 1.903 s)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 1.846 s    (1.834 s .. 1.854 s)
std dev              11.56 ms   (0.0 s .. 13.28 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking print
time                 1.602 s    (1.589 s .. 1.609 s)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 1.607 s    (1.606 s .. 1.607 s)
std dev              352.9 μs   (0.0 s .. 407.5 μs)
variance introduced by outliers: 19% (moderately inflated)

benchmarking register
time                 2.143 s    (2.050 s .. 2.223 s)
                     1.000 R²   (0.999 R² .. 1.000 R²)
mean                 2.135 s    (2.121 s .. 2.146 s)
std dev              17.79 ms   (0.0 s .. 19.60 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking balance
time                 204.6 ms   (196.1 ms .. 209.5 ms)
                     0.999 R²   (0.998 R² .. 1.000 R²)
mean                 204.5 ms   (202.2 ms .. 206.1 ms)
std dev              2.434 ms   (1.329 ms .. 3.665 ms)
variance introduced by outliers: 14% (moderately inflated)

benchmarking stats
time                 190.3 ms   (182.7 ms .. 195.5 ms)
                     0.999 R²   (0.998 R² .. 1.000 R²)
mean                 190.9 ms   (188.6 ms .. 194.4 ms)
std dev              3.729 ms   (696.3 μs .. 5.146 ms)
variance introduced by outliers: 14% (moderately inflated)

Memory usage doesn’t seem to be significantly impacted (although it looks like it goes slightly down with this patch).

@simonmichael simonmichael modified the milestone: 1.1 Dec 29, 2016

@cocreature

This comment has been minimized.

Show comment
Hide comment
@cocreature

cocreature Jan 6, 2017

Collaborator

Bumping because there hasn’t been a response so far.

Collaborator

cocreature commented Jan 6, 2017

Bumping because there hasn’t been a response so far.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 9, 2017

Owner

I'm sorry, I keep meaning to respond to this, but finding other things to do. Will do today or tomorrow.

Owner

simonmichael commented Jan 9, 2017

I'm sorry, I keep meaning to respond to this, but finding other things to do. Will do today or tomorrow.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 11, 2017

Owner

I'd like to benchmark this too, first rebasing it against master so I am comparing like with like, but by now it has two conflicts with master which are not so easy for me to resolve correctly. @cocreature, could you do the rebase ?

Owner

simonmichael commented Jan 11, 2017

I'd like to benchmark this too, first rebasing it against master so I am comparing like with like, but by now it has two conflicts with master which are not so easy for me to resolve correctly. @cocreature, could you do the rebase ?

@cocreature

This comment has been minimized.

Show comment
Hide comment
@cocreature

cocreature Jan 11, 2017

Collaborator

I’ll try rebasing sometime this week.

Collaborator

cocreature commented Jan 11, 2017

I’ll try rebasing sometime this week.

@cocreature

This comment has been minimized.

Show comment
Hide comment
@cocreature

cocreature Jan 12, 2017

Collaborator

@simonmichael rebased.

Collaborator

cocreature commented Jan 12, 2017

@simonmichael rebased.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 12, 2017

Owner

Thanks. Here's what I get:

$ quickbench -n4 -w hledger-master-d92d777c,hledger-413-5bc00818 'hledger -f examples/10000x1000x10.journal stats' 'hledger -f examples/10000x1000x10.journal print' 'hledger -f examples/10000x1000x10.journal balance' 'hledger -f examples/10000x1000x10.journal register'
Running 4 tests 4 times with 2 executables at 2017-01-12 12:48:04 PST:

Best times:
+--------------------------------------------++-------------------------+----------------------+
|                                            || hledger-master-d92d777c | hledger-413-5bc00818 |
+============================================++=========================+======================+
| -f examples/10000x1000x10.journal stats    ||                    2.03 |                 2.07 |
| -f examples/10000x1000x10.journal print    ||                    3.05 |                 3.12 |
| -f examples/10000x1000x10.journal balance  ||                    2.03 |                 2.21 |
| -f examples/10000x1000x10.journal register ||                    4.02 |                 3.94 |
+--------------------------------------------++-------------------------+----------------------+

Or using the benchmark suite:

master:

~/src/hledger$ stack bench hledger
hledger-1.1.98: benchmarks
Running 1 benchmarks...
Benchmark bench: RUNNING...
Benchmarking hledger in /Users/simon/src/hledger/hledger with timeit
read bench/10000x1000x10.journal        [1.63s]
print                                   [1.23s]
register                                [1.67s]
balance                                 [0.15s]
Total: 4.68s
Benchmark bench: FINISH

413:

~/src/hledger$ stack bench hledger
hledger-1.1.98: benchmarks
Running 1 benchmarks...
Benchmark bench: RUNNING...
Benchmarking hledger in /Users/simon/src/hledger/hledger with timeit
read bench/10000x1000x10.journal        [1.73s]
print                                   [1.14s]
register                                [1.72s]
balance                                 [0.15s]
Total: 4.74s
Benchmark bench: FINISH
Completed 3 action(s).

Or with criterion:

master:

~/src/hledger$ stack bench hledger --benchmark-arguments  --criterion
hledger-1.1.98: benchmarks
Running 1 benchmarks...
Benchmark bench: RUNNING...
Benchmarking hledger in /Users/simon/src/hledger/hledger with criterion
benchmarking read bench/10000x1000x10.journal
time                 1.589 s    (1.472 s .. 1.759 s)
                     0.999 R²   (0.996 R² .. 1.000 R²)
mean                 1.620 s    (1.594 s .. 1.644 s)
std dev              39.29 ms   (0.0 s .. 41.16 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking print
time                 1.091 s    (857.4 ms .. 1.385 s)
                     0.992 R²   (0.973 R² .. 1.000 R²)
mean                 1.155 s    (1.111 s .. 1.198 s)
std dev              74.08 ms   (0.0 s .. 74.49 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking register
time                 1.607 s    (1.437 s .. 1.867 s)
                     0.997 R²   (0.992 R² .. 1.000 R²)
mean                 1.679 s    (1.631 s .. 1.721 s)
std dev              67.61 ms   (0.0 s .. 72.42 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking balance
time                 172.2 ms   (162.8 ms .. 187.2 ms)
                     0.993 R²   (0.976 R² .. 1.000 R²)
mean                 172.5 ms   (169.3 ms .. 177.7 ms)
std dev              5.662 ms   (2.580 ms .. 8.277 ms)
variance introduced by outliers: 12% (moderately inflated)

benchmarking stats
time                 141.8 ms   (133.0 ms .. 155.5 ms)
                     0.995 R²   (0.987 R² .. 1.000 R²)
mean                 150.7 ms   (146.1 ms .. 155.2 ms)
std dev              6.634 ms   (4.564 ms .. 8.717 ms)
variance introduced by outliers: 12% (moderately inflated)

Benchmark bench: FINISH

413:

~/src/hledger$ stack bench hledger --benchmark-arguments  --criterion
hledger-1.1.98: benchmarks
Running 1 benchmarks...
Benchmark bench: RUNNING...
Benchmarking hledger in /Users/simon/src/hledger/hledger with criterion
benchmarking read bench/10000x1000x10.journal
time                 1.692 s    (1.091 s .. 2.433 s)
                     0.978 R²   (0.932 R² .. 1.000 R²)
mean                 1.799 s    (1.710 s .. 1.879 s)
std dev              132.3 ms   (271.9 as .. 139.2 ms)
variance introduced by outliers: 20% (moderately inflated)

benchmarking print
time                 1.189 s    (1.061 s .. 1.411 s)
                     0.996 R²   (0.990 R² .. 1.000 R²)
mean                 1.116 s    (1.085 s .. 1.166 s)
std dev              44.08 ms   (0.0 s .. 47.06 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking register
time                 1.512 s    (1.097 s .. 1.994 s)
                     0.988 R²   (0.960 R² .. 1.000 R²)
mean                 1.736 s    (1.633 s .. 1.834 s)
std dev              164.6 ms   (0.0 s .. 169.2 ms)
variance introduced by outliers: 22% (moderately inflated)

benchmarking balance
time                 159.6 ms   (153.6 ms .. 164.1 ms)
                     0.999 R²   (0.997 R² .. 1.000 R²)
mean                 159.9 ms   (157.5 ms .. 162.9 ms)
std dev              3.569 ms   (2.154 ms .. 5.673 ms)
variance introduced by outliers: 12% (moderately inflated)

benchmarking stats
time                 150.9 ms   (144.5 ms .. 157.4 ms)
                     0.998 R²   (0.996 R² .. 1.000 R²)
mean                 160.5 ms   (156.0 ms .. 166.3 ms)
std dev              7.110 ms   (4.594 ms .. 9.727 ms)
variance introduced by outliers: 12% (moderately inflated)

Benchmark bench: FINISH

If anything, it seems slower, as far as I can measure ?

Some of that new code sounds nice to have, but we should probably see a real benefit before giving up the readability and familiarity and low maintenance of standard library functions. I think that also goes for INLINABLE, which we discussed on another PR.

Owner

simonmichael commented Jan 12, 2017

Thanks. Here's what I get:

$ quickbench -n4 -w hledger-master-d92d777c,hledger-413-5bc00818 'hledger -f examples/10000x1000x10.journal stats' 'hledger -f examples/10000x1000x10.journal print' 'hledger -f examples/10000x1000x10.journal balance' 'hledger -f examples/10000x1000x10.journal register'
Running 4 tests 4 times with 2 executables at 2017-01-12 12:48:04 PST:

Best times:
+--------------------------------------------++-------------------------+----------------------+
|                                            || hledger-master-d92d777c | hledger-413-5bc00818 |
+============================================++=========================+======================+
| -f examples/10000x1000x10.journal stats    ||                    2.03 |                 2.07 |
| -f examples/10000x1000x10.journal print    ||                    3.05 |                 3.12 |
| -f examples/10000x1000x10.journal balance  ||                    2.03 |                 2.21 |
| -f examples/10000x1000x10.journal register ||                    4.02 |                 3.94 |
+--------------------------------------------++-------------------------+----------------------+

Or using the benchmark suite:

master:

~/src/hledger$ stack bench hledger
hledger-1.1.98: benchmarks
Running 1 benchmarks...
Benchmark bench: RUNNING...
Benchmarking hledger in /Users/simon/src/hledger/hledger with timeit
read bench/10000x1000x10.journal        [1.63s]
print                                   [1.23s]
register                                [1.67s]
balance                                 [0.15s]
Total: 4.68s
Benchmark bench: FINISH

413:

~/src/hledger$ stack bench hledger
hledger-1.1.98: benchmarks
Running 1 benchmarks...
Benchmark bench: RUNNING...
Benchmarking hledger in /Users/simon/src/hledger/hledger with timeit
read bench/10000x1000x10.journal        [1.73s]
print                                   [1.14s]
register                                [1.72s]
balance                                 [0.15s]
Total: 4.74s
Benchmark bench: FINISH
Completed 3 action(s).

Or with criterion:

master:

~/src/hledger$ stack bench hledger --benchmark-arguments  --criterion
hledger-1.1.98: benchmarks
Running 1 benchmarks...
Benchmark bench: RUNNING...
Benchmarking hledger in /Users/simon/src/hledger/hledger with criterion
benchmarking read bench/10000x1000x10.journal
time                 1.589 s    (1.472 s .. 1.759 s)
                     0.999 R²   (0.996 R² .. 1.000 R²)
mean                 1.620 s    (1.594 s .. 1.644 s)
std dev              39.29 ms   (0.0 s .. 41.16 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking print
time                 1.091 s    (857.4 ms .. 1.385 s)
                     0.992 R²   (0.973 R² .. 1.000 R²)
mean                 1.155 s    (1.111 s .. 1.198 s)
std dev              74.08 ms   (0.0 s .. 74.49 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking register
time                 1.607 s    (1.437 s .. 1.867 s)
                     0.997 R²   (0.992 R² .. 1.000 R²)
mean                 1.679 s    (1.631 s .. 1.721 s)
std dev              67.61 ms   (0.0 s .. 72.42 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking balance
time                 172.2 ms   (162.8 ms .. 187.2 ms)
                     0.993 R²   (0.976 R² .. 1.000 R²)
mean                 172.5 ms   (169.3 ms .. 177.7 ms)
std dev              5.662 ms   (2.580 ms .. 8.277 ms)
variance introduced by outliers: 12% (moderately inflated)

benchmarking stats
time                 141.8 ms   (133.0 ms .. 155.5 ms)
                     0.995 R²   (0.987 R² .. 1.000 R²)
mean                 150.7 ms   (146.1 ms .. 155.2 ms)
std dev              6.634 ms   (4.564 ms .. 8.717 ms)
variance introduced by outliers: 12% (moderately inflated)

Benchmark bench: FINISH

413:

~/src/hledger$ stack bench hledger --benchmark-arguments  --criterion
hledger-1.1.98: benchmarks
Running 1 benchmarks...
Benchmark bench: RUNNING...
Benchmarking hledger in /Users/simon/src/hledger/hledger with criterion
benchmarking read bench/10000x1000x10.journal
time                 1.692 s    (1.091 s .. 2.433 s)
                     0.978 R²   (0.932 R² .. 1.000 R²)
mean                 1.799 s    (1.710 s .. 1.879 s)
std dev              132.3 ms   (271.9 as .. 139.2 ms)
variance introduced by outliers: 20% (moderately inflated)

benchmarking print
time                 1.189 s    (1.061 s .. 1.411 s)
                     0.996 R²   (0.990 R² .. 1.000 R²)
mean                 1.116 s    (1.085 s .. 1.166 s)
std dev              44.08 ms   (0.0 s .. 47.06 ms)
variance introduced by outliers: 19% (moderately inflated)

benchmarking register
time                 1.512 s    (1.097 s .. 1.994 s)
                     0.988 R²   (0.960 R² .. 1.000 R²)
mean                 1.736 s    (1.633 s .. 1.834 s)
std dev              164.6 ms   (0.0 s .. 169.2 ms)
variance introduced by outliers: 22% (moderately inflated)

benchmarking balance
time                 159.6 ms   (153.6 ms .. 164.1 ms)
                     0.999 R²   (0.997 R² .. 1.000 R²)
mean                 159.9 ms   (157.5 ms .. 162.9 ms)
std dev              3.569 ms   (2.154 ms .. 5.673 ms)
variance introduced by outliers: 12% (moderately inflated)

benchmarking stats
time                 150.9 ms   (144.5 ms .. 157.4 ms)
                     0.998 R²   (0.996 R² .. 1.000 R²)
mean                 160.5 ms   (156.0 ms .. 166.3 ms)
std dev              7.110 ms   (4.594 ms .. 9.727 ms)
variance introduced by outliers: 12% (moderately inflated)

Benchmark bench: FINISH

If anything, it seems slower, as far as I can measure ?

Some of that new code sounds nice to have, but we should probably see a real benefit before giving up the readability and familiarity and low maintenance of standard library functions. I think that also goes for INLINABLE, which we discussed on another PR.

@cocreature

This comment has been minimized.

Show comment
Hide comment
@cocreature

cocreature Jan 12, 2017

Collaborator

Some of that new code sounds nice to have, but we should probably see a real benefit before giving up the readability and familiarity and low maintenance of standard library functions. I think that also goes for INLINABLE, which we discussed on another PR.

I disagree with that part. Adding INLINABLE to improve the performance is a nice to have for me. I see spaceleaks as bugs so they always should be fixed.

The differences in the benchmarks look too small to be significant. I would expect that if you run it a few times you’ll see differences in that range in both directions.

Collaborator

cocreature commented Jan 12, 2017

Some of that new code sounds nice to have, but we should probably see a real benefit before giving up the readability and familiarity and low maintenance of standard library functions. I think that also goes for INLINABLE, which we discussed on another PR.

I disagree with that part. Adding INLINABLE to improve the performance is a nice to have for me. I see spaceleaks as bugs so they always should be fixed.

The differences in the benchmarks look too small to be significant. I would expect that if you run it a few times you’ll see differences in that range in both directions.

@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 12, 2017

Owner

Measuring space usage. A small improvement ?:

~/src/hledger$ hledger-master-d92d777c -f examples/10000x1000x10.journal bal >/dev/null +RTS -s
   3,102,398,344 bytes allocated in the heap
     255,161,968 bytes copied during GC
      39,020,984 bytes maximum residency (11 sample(s))
       2,508,128 bytes maximum slop
              89 MB total memory in use (0 MB lost due to fragmentation)
  Total   time    1.932s  (  2.424s elapsed)
  Alloc rate    2,097,112,037 bytes per MUT second
  Productivity  76.8% of total user, 61.2% of total elapsed

~/src/hledger$ hledger-413-5bc00818 -f examples/10000x1000x10.journal bal >/dev/null +RTS -s
   3,102,614,744 bytes allocated in the heap
     284,433,464 bytes copied during GC
      38,750,304 bytes maximum residency (11 sample(s))
       2,508,168 bytes maximum slop
             109 MB total memory in use (0 MB lost due to fragmentation)
  Total   time    1.933s  (  2.255s elapsed)
  Alloc rate    2,113,430,117 bytes per MUT second
  Productivity  76.1% of total user, 65.3% of total elapsed

~/src/hledger$ hledger-413-3936bd8a -f examples/10000x1000x10.journal bal >/dev/null +RTS -s  # 413 without the NOINLINEs
   3,100,854,624 bytes allocated in the heap
     284,218,560 bytes copied during GC
      38,750,304 bytes maximum residency (11 sample(s))
       2,508,168 bytes maximum slop
             109 MB total memory in use (0 MB lost due to fragmentation)
  Total   time    1.927s  (  2.248s elapsed)
  Alloc rate    2,136,435,563 bytes per MUT second
  Productivity  75.5% of total user, 64.7% of total elapsed
Owner

simonmichael commented Jan 12, 2017

Measuring space usage. A small improvement ?:

~/src/hledger$ hledger-master-d92d777c -f examples/10000x1000x10.journal bal >/dev/null +RTS -s
   3,102,398,344 bytes allocated in the heap
     255,161,968 bytes copied during GC
      39,020,984 bytes maximum residency (11 sample(s))
       2,508,128 bytes maximum slop
              89 MB total memory in use (0 MB lost due to fragmentation)
  Total   time    1.932s  (  2.424s elapsed)
  Alloc rate    2,097,112,037 bytes per MUT second
  Productivity  76.8% of total user, 61.2% of total elapsed

~/src/hledger$ hledger-413-5bc00818 -f examples/10000x1000x10.journal bal >/dev/null +RTS -s
   3,102,614,744 bytes allocated in the heap
     284,433,464 bytes copied during GC
      38,750,304 bytes maximum residency (11 sample(s))
       2,508,168 bytes maximum slop
             109 MB total memory in use (0 MB lost due to fragmentation)
  Total   time    1.933s  (  2.255s elapsed)
  Alloc rate    2,113,430,117 bytes per MUT second
  Productivity  76.1% of total user, 65.3% of total elapsed

~/src/hledger$ hledger-413-3936bd8a -f examples/10000x1000x10.journal bal >/dev/null +RTS -s  # 413 without the NOINLINEs
   3,100,854,624 bytes allocated in the heap
     284,218,560 bytes copied during GC
      38,750,304 bytes maximum residency (11 sample(s))
       2,508,168 bytes maximum slop
             109 MB total memory in use (0 MB lost due to fragmentation)
  Total   time    1.927s  (  2.248s elapsed)
  Alloc rate    2,136,435,563 bytes per MUT second
  Productivity  75.5% of total user, 64.7% of total elapsed
@simonmichael

This comment has been minimized.

Show comment
Hide comment
@simonmichael

simonmichael Jan 13, 2017

Owner

Performance is not significantly different, fixing leaks is good, being more amenable to ndmitchell's technique is good, and this code is instructive. Thanks!

Owner

simonmichael commented Jan 13, 2017

Performance is not significantly different, fixing leaks is good, being more amenable to ndmitchell's technique is good, and this code is instructive. Thanks!

@simonmichael simonmichael merged commit d236f7b into simonmichael:master Jan 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

mstksg added a commit to mstksg/hledger that referenced this pull request Feb 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment