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

[SR-4667] Array is probably leaking memory #47244

Open
palimondo mannequin opened this issue Apr 22, 2017 · 11 comments
Open

[SR-4667] Array is probably leaking memory #47244

palimondo mannequin opened this issue Apr 22, 2017 · 11 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. memory leak bug: Memory leak standard library Area: Standard library umbrella

Comments

@palimondo
Copy link
Mannequin

palimondo mannequin commented Apr 22, 2017

Previous ID SR-4667
Radar rdar://problem/31788786
Original Reporter @palimondo
Type Bug
Environment

swift[master] at the time of this report

Additional Detail from JIRA
Votes 0
Component/s Standard Library
Labels Bug, Leak
Assignee None
Priority Medium

md5: 9f5c7a890e0ddedf4872a7fdbb24d9e6

relates to:

  • SR-4666 Dictionary bridge is leaking memory

Issue Description:

Running MonteCarloE performance test from Swift Benchmark Suite reveals unstable memory usage in Array:

$ /usr/bin/time -lp $BUILD/bin/Benchmark_O --num-samples=1 MonteCarloE
[...]
  17092608  maximum resident set size
[...]
$ /usr/bin/time -lp $BUILD/bin/Benchmark_O --num-samples=3 MonteCarloE
[...]
  18915328  maximum resident set size
$ /usr/bin/time -lp $BUILD/bin/Benchmark_O --num-samples=20 MonteCarloE
[...]
  22515712  maximum resident set size
[...]
$ /usr/bin/time -lp $BUILD/bin/Benchmark_O --num-samples=100 MonteCarloE
[...]
  23134208  maximum resident set size
number of iterations MAX_RSS
1 17 MB
3 18 MB
20 22 MB
100 23 MB

It appears that raising the num-samples tends to give higher MAX_RSS, but the reported value varies drastically between runs with the same num-samples.

samples MAX_RSS (B)
1 16904192
1 12492800
1 16707584
1 13897728
1 16883712

Same problem manifests when running benchmarks for Array2D, ArrayAppendSequence, ArrayAppend, ArrayAppendStrings, ArrayPlusEqualSingleElementCollection, ArraySubscript.

@belkadan
Copy link
Contributor

@swift-ci create

@aschwaighofer
Copy link
Member

I don’t think you can use Resident Set Size as an indicator for leaks. If we keep asking and freeing memory the OS does necessearily need to reclaim those map pages until it is under memory pressure … 22MB is probably not creating that pressure.

@palimondo
Copy link
Mannequin Author

palimondo mannequin commented Apr 24, 2017

Did you mean to say "OS does not necessarily need to reclaim"?

What explains the variance (12MB vs 17MB) between runs?

@aschwaighofer
Copy link
Member

Yes I meant to say "does not necessarily need to reclaim".

@palimondo
Copy link
Mannequin Author

palimondo mannequin commented Apr 26, 2017

Given that MAX_RSS is not good enough to detect leaks, I’m not insisting it is a leak. I’m trying to get the performance tests to behave deterministically – consistent memory usage between runs seems like a sensible requirement to me.

What explains the huge variance in MAX_RSS between runs here? 12 MB vs 17 MB, identical number of iterations. I’m pointing at MonteCarloE as most exacerbated example, but all the Array benchmarks have this instability. That’s my core concern. Leak was just a guess.

@aschwaighofer
Copy link
Member

Try running with a consistent number of iterations (vs. samples).

The benchmark suite will run a test for varying number of iterations if you don't to that.

$ /usr/bin/time -lp $PWD/bin/Benchmark_O --num-iters=110 MonteCarloE 2>&1 | grep max
  46313472  maximum resident set size
$ /usr/bin/time -lp $PWD/bin/Benchmark_O --num-iters=110 MonteCarloE 2>&1 | grep max
  46313472  maximum resident set size
$ /usr/bin/time -lp $PWD/bin/Benchmark_O --num-iters=110 MonteCarloE 2>&1 | grep max
  46313472  maximum resident set size
$ /usr/bin/time -lp $PWD/bin/Benchmark_O --num-iters=110 MonteCarloE 2>&1 | grep max
  46313472  maximum resident set size
$ /usr/bin/time -lp $PWD/bin/Benchmark_O --num-iters=110 MonteCarloE 2>&1 | grep max
  46309376  maximum resident set size
$ /usr/bin/time -lp $PWD/bin/Benchmark_O --num-iters=110 MonteCarloE 2>&1 | grep max
  46313472  maximum resident set size

@aschwaighofer
Copy link
Member

--verbose will show you how many iterations it runs (I don't know why we called it "scale" in the verbose output)

$ bin/Benchmark_O --verbose MonteCarloE
Verbose
--- CONFIG ---
NumSamples: 1
Verbose: true
IterScale: 1
Tests Filter: ["MonteCarloE"]
Tests to run: MonteCarloE, 

--- DATA ---
#,TEST,SAMPLES,MIN(us),MAX(us),MEAN(us),SD(us),MEDIAN(us)
Running MonteCarloE for 1 samples.
    Measuring with scale 109.
    Sample 0,9086
124,MonteCarloE,1,9086,9086,9086,0,9086

Totals,1,9086,9086,9086,0,0
$ bin/Benchmark_O --verbose MonteCarloE
Verbose
--- CONFIG ---
NumSamples: 1
Verbose: true
IterScale: 1
Tests Filter: ["MonteCarloE"]
Tests to run: MonteCarloE, 

--- DATA ---
#,TEST,SAMPLES,MIN(us),MAX(us),MEAN(us),SD(us),MEDIAN(us)
Running MonteCarloE for 1 samples.
    Measuring with scale 106.
    Sample 0,9141
124,MonteCarloE,1,9141,9141,9141,0,9141

Totals,1,9141,9141,9141,0,0

@palimondo
Copy link
Mannequin Author

palimondo mannequin commented Apr 26, 2017

Wow! Thank you, that explains the variance between run.

I read somewhere that Benchmark_0 will time 1 iteration and derive N to make it run for approximately 1 second. So between different invocations, it is choosing different Ns, because the timing baseline varies, too.

So, the whole business with averaging MAX_RSS is really misguided - this (combined with delayed memory releases for autoreleased objects) is the source of variance. Different number of iterations will accumulate different amount of garbage in the pool - which manifests as different MAX_RSS between runs.
🤦

@palimondo
Copy link
Mannequin Author

palimondo mannequin commented Apr 26, 2017

Still… am I correct in assuming that this issue exist only in code that interoperates with ObjC? Or does pure Swift also have some autoreleased objects?

@aschwaighofer
Copy link
Member

autoreleased objects come from having to deal with the return calling convention of objective c apis. They return their references at +0 (unowned) but because they need to relinquish ownership but at the same time keep the object alive for the duration of the call they put the object into the autorelease pool for deferred release.

@palimondo
Copy link
Mannequin Author

palimondo mannequin commented Apr 27, 2017

And Array is an ObjC object abiding by this convention under the hood?

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. memory leak bug: Memory leak standard library Area: Standard library umbrella
Projects
None yet
Development

No branches or pull requests

2 participants