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.

Sign up for GitHub

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

Jump to bottom

memory leak with coforall reducing an array #13569

Closed
mppf opened this issue Jul 29, 2019 · 1 comment · Fixed by #13604
Closed

memory leak with coforall reducing an array #13569

mppf opened this issue Jul 29, 2019 · 1 comment · Fixed by #13604
Assignees
@vasslitvinov
Labels
area: Compiler type: Bug
Milestone
PB Sprint 35

Comments

@mppf
Copy link
Member

mppf commented Jul 29, 2019 •
edited
Loading

The following program leaks memory:

config const n = 100;
  
proc main() {
  var counts:[0..#n] int;
  var nTasks = 10;
  coforall tid in 0..#nTasks with (+ reduce counts) {
    counts[tid] += 1;
  }
}

This caused an increase in memory leaks in nightly:

https://chapel-lang.org/perf/chapcs/?startdate=2019/07/14&enddate=2019/07/28&graphs=memoryleaksforalltests

from the test test/library/packages/Sort/correctness/test-radix-bucketizer.chpl which calls SequentialInPlacePartitioning.bucketize which uses this pattern. (These were added in PR #13347).
@bradcray
Copy link
Member

Thanks for picking this up, Vass.

@vasslitvinov vasslitvinov added this to the PB Sprint 35 milestone Jul 30, 2019
vasslitvinov added a commit to vasslitvinov/chapel that referenced this issue Aug 1, 2019
@vasslitvinov
Fix memory leaks in test-radix-bucketizer.chpl
99cc54d 
Resolves chapel-lang#13569

When setting up the AST for a task reduce intent, I was using
PRIM_ASSIGN instead of calling the `=` function (which is a really bad
idea when used on an array btw) to store the result of globalOp.generate()
into the outer variable.  Also, the task-private shadow variables were not
getting autoDestroy-ed. This fixes these bugs.
@vasslitvinov vasslitvinov mentioned this issue Aug 1, 2019
Merged
3 tasks
vasslitvinov added a commit that referenced this issue Aug 1, 2019
@vasslitvinov
Merge pull request #13604 from vasslitvinov/fix-task-reduce-memleaks
31640bd 
Fix memory leaks in test-radix-bucketizer.chpl

Resolves #13569, plugging the memory leak in

    test/library/packages/Sort/correctness/test-radix-bucketizer.chpl

When setting up the AST for a task reduce intent, I was using
PRIM_ASSIGN instead of calling the `=` function (which is a really bad
idea when used on an array btw) to store the result of globalOp.generate()
into the outer variable.  Also, the task-private shadow variables were not
getting autoDestroy-ed. This fixes these bugs.

While there, add multi-arg versions of `Expr::insertAfter(Expr*)`, for convenience.
A single varargs version instead would be even nicer.

r: @mppf
vasslitvinov added a commit to vasslitvinov/chapel that referenced this issue Aug 2, 2019
@vasslitvinov
Add a reproducer for chapel-lang#13569
4cde973
@vasslitvinov vasslitvinov mentioned this issue Aug 2, 2019
Merged
vasslitvinov added a commit that referenced this issue Aug 2, 2019
@vasslitvinov
Merge pull request #13636 from vasslitvinov/add-coforall-reduce-example
eaea45c 
Add a reproducer for #13569

While #13569 was fixed in #13604, we would like to have a dedicated
small reproducer to ensure that the issue stays in check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vasslitvinov vasslitvinov

Labels
area: Compiler type: Bug
Projects
None yet
Milestone
PB Sprint 35
Development

Successfully merging a pull request may close this issue.

Fix memory leaks in test-radix-bucketizer.chpl vasslitvinov/chapel
3 participants
@mppf @bradcray @vasslitvinov