CEMeNT-PSAAP / MCDC

MC/DC: Monte Carlo Dynamic Code
https://mcdc.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
23 stars 24 forks source link

Early Memory Deallocation/Re-Use in Local Arrays #224

Open braxtoncuneo opened 2 months ago

braxtoncuneo commented 2 months ago

While working on the AMD functionality, I came across an issue in Numba. I'm also filing an issue with the Numba folks, but until they get back, be advised that memory associated with arrays can (apparently) be freed and re-used before the last reference to its content is used.

Here's a minimum viable reproduction:

import numba
import numpy as np

kind = np.dtype([("member", np.int64)])

@numba.njit()
def func_a(param_record):
    local_record = np.zeros(1,kind)[0]
    local_record["member"] = 12345
    print(param_record["member"])

@numba.njit()
def func_b():
    local_record = np.zeros(1,kind)[0]
    local_record["member"] = 67890
    func_a(local_record)

func_b()

This program, which should print 67890 (the value most recently assigned to func_b's local_record), actually prints 12345 (the value most recently assigned to func_a's local_record). It looks like Numba thinks we aren't using the local_record from func_b anymore because we are passing one of its elements, rather than the whole thing. The memory associated with func_b's local_record gets re-used as the local_record for func_a, and now we have two separate variables seemingly occupying the same memory.

Here is a fix that works for the mvr:

import numba
import numpy as np

kind = np.dtype([("member", np.int64)])

@numba.njit()
def func_a(param_record):
    local_record = np.zeros(1,kind)[0]
    local_record["member"] = 12345
    print(param_record[0]["member"])

@numba.njit()
def func_b():
    local_record = np.zeros(1,kind)
    local_record[0]["member"] = 67890
    func_a(local_record)

func_b()

It's a bit clunky, but it may be our only option in the short term. I'm tweaking my AMD branch to match this fix. I'll let you folks know how it goes, in case you want to perform similar changes to dev ahead of the merge.

ilhamv commented 2 months ago

Are the ones currently in MC/DC (the local_*s) good, @braxtoncuneo?

clemekay commented 2 months ago

Linking to Braxton's submitted numba issue: https://github.com/numba/numba/issues/9698

braxtoncuneo commented 2 months ago

Are the ones currently in MC/DC (the local_*s) good, @braxtoncuneo?

Not exactly. Those functions were written under the idea that the values passed out of those functions are passed out by value. This bug implies that records always refer to their originating array, and that arrays do not persist in other scopes unless those arrays are passed through.

I'm working on possible workarounds. My current best candidate is declaring arrays through inlined overloads/intrinsics/builtins.