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

Add del's to reductions. #5047

Closed
wants to merge 6 commits into from
Closed

Add del's to reductions. #5047

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b8db073
Fix in progress.
DrTodd13 Jan 7, 2020
d8cff6b
Fix reduction on array memory leak.
DrTodd13 Jan 7, 2020
a69e294
Remove debug print
DrTodd13 Jan 7, 2020
8c1b924
Remove print.
DrTodd13 Jan 7, 2020
00d3ed1
Don't do a ir.Del for the last inst in the inst set. Move the ir.Del…
DrTodd13 Jan 7, 2020
96ec1e3
Remove experimental ir.Del call in call_parallel_gufunc.
DrTodd13 Jan 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion numba/npyufunc/parfor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ def _lower_parfor_parallel(lowerer, parfor):
# init reduction array allocation here.
nredvars = len(parfor_redvars)
redarrs = {}
redtoset_dels = []
if nredvars > 0:
# reduction arrays outer dimension equal to thread count
thread_count = get_thread_count()
Expand All @@ -114,7 +115,7 @@ def _lower_parfor_parallel(lowerer, parfor):
redarrvar_typ = redtyp_to_redarraytype(redvar_typ)
reddtype = redarrvar_typ.dtype
if config.DEBUG_ARRAY_OPT:
print("redvar_typ", redvar_typ, redarrvar_typ, reddtype, types.DType(reddtype))
print("redvar_typ", redvar, redvar_typ, redarrvar_typ, reddtype, types.DType(reddtype))

# If this is reduction over an array,
# the reduction array has just one added per-worker dimension.
Expand Down Expand Up @@ -204,6 +205,7 @@ def _lower_parfor_parallel(lowerer, parfor):
full_call = ir.Expr.call(full_func, [redshape_var, init_val_var], {}, loc=loc)
lowerer.fndesc.calltypes[full_call] = full_sig
redtoset = ir.Var(scope, mk_unique_var("redtoset"), loc)
redtoset_dels.append(redtoset)
redtoset_assign = ir.Assign(full_call, redtoset, loc)
typemap[redtoset.name] = redvar_typ
lowerer.lower_inst(redtoset_assign)
Expand Down Expand Up @@ -395,6 +397,7 @@ def _lower_parfor_parallel(lowerer, parfor):
# Add calltype back in for the expr with updated signature.
lowerer.fndesc.calltypes[rhs] = ct
lowerer.lower_inst(inst)

if isinstance(inst, ir.Assign) and name == inst.target.name:
break

Expand All @@ -418,10 +421,20 @@ def _lower_parfor_parallel(lowerer, parfor):
print("res_print2", res_print)
lowerer.lower_inst(res_print)

lowerer.lower_inst(ir.Del(oneelem.name, loc=loc))
lowerer.lower_inst(ir.Del(init_var.name, loc=loc))

for i in range(nredvars):
name = parfor_redvars[i]
for inst in parfor_reddict[name][1][:-1]:
if isinstance(typemap[inst.target.name], types.npytypes.Array):
lowerer.lower_inst(ir.Del(inst.target.name, loc=loc))

# Cleanup reduction variable
for v in redarrs.values():
lowerer.lower_inst(ir.Del(v.name, loc=loc))
for v in redtoset_dels:
lowerer.lower_inst(ir.Del(v.name, loc=loc))
# Restore the original typemap of the function that was replaced temporarily at the
# Beginning of this function.
lowerer.fndesc.typemap = orig_typemap
Expand Down Expand Up @@ -1539,4 +1552,7 @@ def load_range(v):
only_elem_ptr = builder.gep(rv_arg, [context.get_constant(types.intp, 0)])
builder.store(builder.load(only_elem_ptr), lowerer.getvar(k))

# for v in redarrdict.values():
# lowerer.lower_inst(ir.Del(v.name, loc=loc))

Copy link
Contributor

Choose a reason for hiding this comment

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

dead code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the code that made me want to look at the PR again. Want to make sure the leak appears to be gone. I also had the same question about how to test this and I don't know the answer. @stuartarchibald

Copy link
Contributor

Choose a reason for hiding this comment

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

ah ok, thanks. I'll put working out how to test this on my TODOs, I'm sure we can come up with something, even if we have to write something new it'd probably prove quite useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed those commented lines.

context.active_code_library.add_linking_library(cres.library)