Fix leak in write_nrnthread.

[1uc]

Running a memory profiler on simulations using neurodamus to create the input files for coreneuron in multiple cycles showed that memory allocated in:

nrn/src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp

Line 358 in 5ea534f

data = new double[n * sz];

accumulated across cycles. The function is called from write_nrnthread, with the following context:

        double* data = NULL;
        nrnthread_dat2_mech(..., data, ...);

        if (nrn_is_artificial_[type]) {
            delete[] data;
        }

looking into nrnthread_dat2_mech we find the following structure:

int nrnthread_dat2_mech(..., double*& data, ...) {
    bool copy = data ? true : false;

    // As the NEURON data is now transposed then for now always create a new
    // copy in the format expected by CoreNEURON -- TODO make sure this is
    // freed, then later TODO remove the need for this entirely
    if (!copy) {
        data = new double[n * sz];
    }
    // Don't give away/store `data` anywhere.

In summary when called from write_nrnthread:

1. The argument data passed to nrnthread_dat2_mech is unconditionally NULL.
2. Memory will be allocate to data unconditionally.
3. Memory is only released when nrn_is_artificial_[type] which seems to only sometimes hold.

The immediate fix is to simply release the memory since it doesn't seem to be used afterwards. Profiling the same run shows the expected reduced memory consumption.

[azure-pipelines]

✔️ cd3c5fb -> Azure artifacts URL

[codecov]

Codecov Report

Merging #2456 (932cd32) into master (efc28fe) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2456   +/-   ##
=======================================
  Coverage   60.41%   60.42%           
=======================================
  Files         626      626           
  Lines      120812   120799   -13     
=======================================
+ Hits        72990    72992    +2     
+ Misses      47822    47807   -15     

Files Changed	Coverage Δ
...rniv/nrncore_write/callbacks/nrncore_callbacks.cpp	93.38% <ø> (ø)
src/nrniv/nrncore_write/io/nrncore_io.h	100.00% <ø> (ø)
src/nrniv/nrncore_write/io/nrncore_io.cpp	76.76% <100.00%> (+3.35%)	⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ohm314]

Great find, @1uc ! The fix looks good to me, but I prefer if also someone else looks at it.

[azure-pipelines]

✔️ 0e7d4a2 -> Azure artifacts URL

[bbpbuildbot]

Logfiles from GitLab pipeline #145424 (:no_entry:) have been uploaded here!

Status and direct links:

• ✅ mac_m1_cmake_build: [cmake, ON, OFF, OFF, address]
• ✅ spack_setup
• ✅ build:nmodl
• ✅ build:neuron:nmodl:intel:legacy
• ✅ build:neuron:nmodl:intel:shared
• ✅ build:neuron:nmodl:nvhpc:acc:legacy
• ✅ build:neuron:nmodl:nvhpc:acc:shared
• ✅ build:neuron:nmodl:nvhpc:omp:legacy
• ✅ build:neuron:nmodl:nvhpc:omp
• ✅ test:neuron:nmodl:intel:legacy
• ✅ test:neuron:nmodl:intel:shared
• ⛔ test:neuron:nmodl:nvhpc:acc:legacy
• ⛔ test:neuron:nmodl:nvhpc:acc:shared
• ⛔ test:neuron:nmodl:nvhpc:omp:legacy
• ⛔ test:neuron:nmodl:nvhpc:omp

[azure-pipelines]

✔️ 932cd32 -> Azure artifacts URL

[olupton]

Just to provide some more explicit explanation, before #2027 we had this logic:

nrn/src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp

Lines 354 to 360 in b4ade55

	if (isart) { // data may not be contiguous
	data1 = contiguous_art_data(ml->_data, n, sz); // delete after use
	nodeindices = NULL;
	} else {
	nodeindices = ml->nodeindices; // allocated below if copy
	data1 = ml->_data[0]; // do not delete after use
	}

where contiguous_art_data allocated new storage that needed to be freed later:

nrn/src/nrniv/nrncore_write/io/nrncore_io.cpp

Lines 317 to 326 in efc28fe

	double* contiguous_art_data(Memb_list* ml, int nitem, int szitem) {
	double* d1 = new double[nitem * szitem];
	int k = 0;
	for (int i = 0; i < nitem; ++i) {
	for (int j = 0; j < szitem; ++j) {
	d1[k++] = ml->data(i, j);
	}
	}
	return d1;
	}

whereas in master we treat artificial and non-artificial cells uniformly:

nrn/src/nrniv/nrncore_write/callbacks/nrncore_callbacks.cpp

Lines 354 to 359 in efc28fe

	// As the NEURON data is now transposed then for now always create a new
	// copy in the format expected by CoreNEURON -- TODO make sure this is
	// freed, then later TODO remove the need for this entirely
	if (!copy) {
	data = new double[n * sz];
	}