From f60bf591e431df3752a0d0e2a04ea5485514c416 Mon Sep 17 00:00:00 2001 From: Ryan Curtin Date: Wed, 28 Sep 2022 18:38:34 -0400 Subject: [PATCH] Clean up memory handling: ensure Julia GC doesn't double-free. --- src/mlpack/bindings/julia/julia_util.cpp | 41 ++-- src/mlpack/bindings/julia/julia_util.h | 19 +- src/mlpack/bindings/julia/mlpack/params.jl.in | 182 ++++++++++++++---- .../julia/print_input_processing_impl.hpp | 11 +- src/mlpack/bindings/julia/print_jl.cpp | 6 + .../julia/print_output_processing_impl.hpp | 4 +- src/mlpack/bindings/julia/tests/runtests.jl | 23 +-- 7 files changed, 206 insertions(+), 80 deletions(-) diff --git a/src/mlpack/bindings/julia/julia_util.cpp b/src/mlpack/bindings/julia/julia_util.cpp index 1e7781951f..0f1819496a 100644 --- a/src/mlpack/bindings/julia/julia_util.cpp +++ b/src/mlpack/bindings/julia/julia_util.cpp @@ -128,7 +128,7 @@ void SetParamVectorStrStr(void* params, */ void SetParamVectorInt(void* params, const char* paramName, - int* ints, + long long* ints, const size_t length) { util::Params* p = (util::Params*) params; @@ -138,7 +138,7 @@ void SetParamVectorInt(void* params, std::vector vec; vec.resize(length); for (size_t i = 0; i < length; ++i) - vec[i] = ints[i]; + vec[i] = int(ints[i]); p->Get>(paramName) = std::move(vec); p->SetPassed(paramName); @@ -164,10 +164,13 @@ void SetParamMat(void* params, /** * Call params.SetParam>(). + * + * Note that we will have to allocate memory, since we must convert to a size_t + * matrix, and we also subtract by one (since Julia uses 1-indexed labels). */ void SetParamUMat(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t rows, const size_t cols, const bool pointsAsRows) @@ -175,10 +178,11 @@ void SetParamUMat(void* params, util::Params* p = (util::Params*) params; // Create the matrix as an alias. - arma::Mat m(memptr, arma::uword(rows), arma::uword(cols), false, - false); - p->Get>(paramName) = pointsAsRows ? m.t() : - std::move(m); + arma::Mat m(memptr, arma::uword(rows), arma::uword(cols), true, + true); + arma::Mat convM = arma::conv_to>::from(m - 1); + p->Get>(paramName) = pointsAsRows ? convM.t() : + std::move(convM); p->SetPassed(paramName); } @@ -201,12 +205,13 @@ void SetParamRow(void* params, */ void SetParamURow(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t cols) { util::Params* p = (util::Params*) params; - arma::Row m(memptr, arma::uword(cols), false, false); - p->Get>(paramName) = std::move(m); + arma::Row m(memptr, arma::uword(cols), false, true); + arma::Row convM = arma::conv_to>::from(m - 1); + p->Get>(paramName) = std::move(convM); p->SetPassed(paramName); } @@ -229,12 +234,13 @@ void SetParamCol(void* params, */ void SetParamUCol(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t rows) { util::Params* p = (util::Params*) params; - arma::Col m(memptr, arma::uword(rows), false, false); - p->Get>(paramName) = std::move(m); + arma::Col m(memptr, arma::uword(rows), false, true); + arma::Col convM = arma::conv_to>::from(m - 1); + p->Get>(paramName) = std::move(convM); p->SetPassed(paramName); } @@ -365,14 +371,17 @@ size_t GetParamVectorIntLen(void* params, const char* paramName) * The vector will be created in-place and it is expected that the calling * function will take ownership. */ -int* GetParamVectorIntPtr(void* params, const char* paramName) +long long* GetParamVectorIntPtr(void* params, const char* paramName) { util::Params* p = (util::Params*) params; const size_t size = p->Get>(paramName).size(); - int* ints = new int[size]; + if (size == 0) + return NULL; + + long long* ints = new long long[size]; for (size_t i = 0; i < size; ++i) - ints[i] = p->Get>(paramName)[i]; + ints[i] = (long long)(p->Get>(paramName)[i]); return ints; } diff --git a/src/mlpack/bindings/julia/julia_util.h b/src/mlpack/bindings/julia/julia_util.h index b990678d7f..b3e01620b1 100644 --- a/src/mlpack/bindings/julia/julia_util.h +++ b/src/mlpack/bindings/julia/julia_util.h @@ -84,7 +84,7 @@ void SetParamVectorStrStr(void* params, */ void SetParamVectorInt(void* params, const char* paramName, - int* ints, + long long* ints, const size_t length); /** @@ -99,10 +99,13 @@ void SetParamMat(void* params, /** * Call params.SetParam>(). + * + * Note that we will have to allocate memory, since we must convert to a size_t + * matrix, and we also subtract by one (since Julia uses 1-indexed labels). */ void SetParamUMat(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t rows, const size_t cols, const bool pointsAsRows); @@ -117,10 +120,13 @@ void SetParamRow(void* params, /** * Call params.SetParam>(). + * + * Note that we will have to allocate memory, since we must convert to a size_t + * vector, and we also subtract by one (since Julia uses 1-indexed labels). */ void SetParamURow(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t cols); /** @@ -133,10 +139,13 @@ void SetParamCol(void* params, /** * Call params.SetParam>(). + * + * Note that we will have to allocate memory, since we must convert to a size_t + * vector, and we also subtract by one (since Julia uses 1-indexed labels). */ void SetParamUCol(void* params, const char* paramName, - size_t* memptr, + long long* memptr, const size_t rows); /** @@ -193,7 +202,7 @@ size_t GetParamVectorIntLen(void* params, const char* paramName); * The vector will be created in-place and it is expected that the calling * function will take ownership. */ -int* GetParamVectorIntPtr(void* params, const char* paramName); +long long* GetParamVectorIntPtr(void* params, const char* paramName); /** * Get the number of rows in a matrix parameter. diff --git a/src/mlpack/bindings/julia/mlpack/params.jl.in b/src/mlpack/bindings/julia/mlpack/params.jl.in index bf3309c1a8..53c490d5ba 100644 --- a/src/mlpack/bindings/julia/mlpack/params.jl.in +++ b/src/mlpack/bindings/julia/mlpack/params.jl.in @@ -103,7 +103,9 @@ end function SetParamMat(params::Ptr{Nothing}, paramName::String, paramValue, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramMat = to_matrix(paramValue, Float64) ccall((:SetParamMat, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Float64}, Csize_t, Csize_t, Bool), params, paramName, Base.pointer(paramMat), @@ -113,7 +115,9 @@ end function SetParamUMat(params::Ptr{Nothing}, paramName::String, paramValue, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramMat = to_matrix(paramValue, Int) # Sanity check. @@ -122,10 +126,11 @@ function SetParamUMat(params::Ptr{Nothing}, "Must be 1 or greater.")) end - m = convert(Array{Csize_t, 2}, paramMat .- 1) - ccall((:SetParamUMat, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Csize_t}, - Csize_t, Csize_t, Bool), params, paramName, Base.pointer(m), - size(paramValue, 1), size(paramValue, 2), pointsAsRows) + # Conversion (and subtracting 1 from the labels) happens in :SetParamUMat. + ccall((:SetParamUMat, library), Nothing, (Ptr{Nothing}, Cstring, + Ptr{Clonglong}, Csize_t, Csize_t, Bool), params, paramName, + Base.pointer(paramMat), size(paramValue, 1), size(paramValue, 2), + pointsAsRows) end function SetParam(params::Ptr{Nothing}, @@ -146,16 +151,18 @@ end function SetParam(params::Ptr{Nothing}, paramName::String, vector::Vector{Int}) - cint_vec = convert(Vector{Cint}, vector) + cintVec = convert(Vector{Clonglong}, vector) ccall((:SetParamVectorInt, library), Nothing, (Ptr{Nothing}, Cstring, - Ptr{Cint}, Csize_t), params, paramName, Base.pointer(cint_vec), - size(cint_vec, 1)) + Ptr{Clonglong}, Csize_t), params, paramName, Base.pointer(cintVec), + size(cintVec, 1)) end function SetParam(params::Ptr{Nothing}, paramName::String, matWithInfo::Tuple{Array{Bool, 1}, Array{Float64, 2}}, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(matWithInfo[2]))) ccall((:SetParamMatWithInfo, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Bool}, Ptr{Float64}, Int, Int, Bool), params, paramName, Base.pointer(matWithInfo[1]), Base.pointer(matWithInfo[2]), @@ -164,7 +171,9 @@ end function SetParamRow(params::Ptr{Nothing}, paramName::String, - paramValue) + paramValue, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramVec = to_vector(paramValue, Float64) ccall((:SetParamRow, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Float64}, Csize_t), params, paramName, Base.pointer(paramVec), size(paramVec, 1)) @@ -172,7 +181,9 @@ end function SetParamCol(params::Ptr{Nothing}, paramName::String, - paramValue) + paramValue, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramVec = to_vector(paramValue, Float64) ccall((:SetParamCol, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Float64}, Csize_t), params, paramName, Base.pointer(paramVec), size(paramVec, 1)) @@ -180,7 +191,9 @@ end function SetParamURow(params::Ptr{Nothing}, paramName::String, - paramValue) + paramValue, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramVec = to_vector(paramValue, Int) # Sanity check. @@ -188,15 +201,18 @@ function SetParamURow(params::Ptr{Nothing}, throw(DomainError("Input $(paramName) cannot have 0 or negative values! " * "Must be 1 or greater.")) end - m = convert(Array{Csize_t, 1}, paramVec .- 1) - ccall((:SetParamURow, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Csize_t}, - Csize_t), params, paramName, Base.pointer(m), size(paramValue, 1)) + # Conversion (and subtracting 1 from the labels) happens in :SetParamURow. + ccall((:SetParamURow, library), Nothing, (Ptr{Nothing}, Cstring, + Ptr{Clonglong}, Csize_t), params, paramName, Base.pointer(paramVec), + size(paramValue, 1)) end function SetParamUCol(params::Ptr{Nothing}, paramName::String, - paramValue) + paramValue, + juliaOwnedMemory::Set{Ptr{Nothing}}) + push!(juliaOwnedMemory, convert(Ptr{Nothing}, Base.pointer(paramValue))) paramVec = to_vector(paramValue, Int) # Sanity check. @@ -204,10 +220,11 @@ function SetParamUCol(params::Ptr{Nothing}, throw(DomainError("Input $(paramName) cannot have 0 or negative values! " * "Must be 1 or greater.")) end - m = convert(Array{Csize_t, 1}, paramValue .- 1) - ccall((:SetParamUCol, library), Nothing, (Ptr{Nothing}, Cstring, Ptr{Csize_t}, - Csize_t), params, paramName, Base.pointer(m), size(paramValue, 1)) + # Conversion (and subtracting 1 from the labels) happens in :SetParamUCol. + ccall((:SetParamUCol, library), Nothing, (Ptr{Nothing}, Cstring, + Ptr{Clonglong}, Csize_t), params, paramName, Base.pointer(paramVec), + size(paramValue, 1)) end function GetParamBool(params::Ptr{Nothing}, paramName::String) @@ -238,6 +255,10 @@ function GetParamVectorStr(params::Ptr{Nothing}, paramName::String) size = ccall((:GetParamVectorStrLen, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) out = Array{String, 1}() + if size == 0 + return out + end + for i = 1:size s = ccall((:GetParamVectorStrStr, library), Cstring, (Ptr{Nothing}, Cstring, Csize_t), params, paramName, i .- 1) @@ -249,22 +270,28 @@ end function GetParamVectorInt(params::Ptr{Nothing}, paramName::String) local size::Csize_t - local ptr::Ptr{Cint} + local ptr::Ptr{Clonglong} # Get the size of the vector, then the pointer to it. We will own the # pointer. size = ccall((:GetParamVectorIntLen, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) - ptr = ccall((:GetParamVectorIntPtr, library), Ptr{Cint}, (Ptr{Nothing}, + # Shortcut: if the vector is empty, no need to allocate. + if size == 0 + return Int[] + end + + ptr = ccall((:GetParamVectorIntPtr, library), Ptr{Clonglong}, (Ptr{Nothing}, Cstring,), params, paramName) - return convert(Array{Int, 1}, Base.unsafe_wrap(Array{Cint, 1}, ptr, (size), - own=true)) + return convert(Array{Int, 1}, Base.unsafe_wrap(Array{Clonglong, 1}, ptr, + (size), own=true)) end function GetParamMat(params::Ptr{Nothing}, paramName::String, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) # Can we return different return types? For now let's restrict to a matrix to # make it easy... local ptr::Ptr{Float64} @@ -275,22 +302,32 @@ function GetParamMat(params::Ptr{Nothing}, params, paramName) cols = ccall((:GetParamMatCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the matrix is empty, return an empty matrix. + if rows == 0 && cols == 0 + return Array{Float64, 2}(undef, 0, 0) + end + ptr = ccall((:GetParamMat, library), Ptr{Float64}, (Ptr{Nothing}, Cstring,), params, paramName) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + if pointsAsRows # In this case we have to transpose, unfortunately. - m = Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=true) + m = Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=own) return m' else # Here no transpose is necessary. - return Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=true) + return Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=own) end end function GetParamUMat(params::Ptr{Nothing}, paramName::String, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) # Can we return different return types? For now let's restrict to a matrix to # make it easy... local ptr::Ptr{Csize_t} @@ -301,73 +338,127 @@ function GetParamUMat(params::Ptr{Nothing}, params, paramName) cols = ccall((:GetParamUMatCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the matrix is empty, return an empty matrix. + if rows == 0 && cols == 0 + return Array{Int, 2}(undef, 0, 0) + end + ptr = ccall((:GetParamUMat, library), Ptr{Csize_t}, (Ptr{Nothing}, Cstring,), params, paramName) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + if pointsAsRows # In this case we have to transpose, unfortunately. - m = Base.unsafe_wrap(Array{Csize_t, 2}, ptr, (rows, cols), own=true) + m = Base.unsafe_wrap(Array{Csize_t, 2}, ptr, (rows, cols), own=own) return convert(Array{Int, 2}, m' .+ 1) # Add 1 because these are indexes. else # Here no transpose is necessary. - m = Base.unsafe_wrap(Array{Csize_t, 2}, ptr, (rows, cols), own=true) + m = Base.unsafe_wrap(Array{Csize_t, 2}, ptr, (rows, cols), own=own) return convert(Array{Int, 2}, m .+ 1) end end -function GetParamCol(params::Ptr{Nothing}, paramName::String) +function GetParamCol(params::Ptr{Nothing}, + paramName::String, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptr::Ptr{Float64} local rows::Csize_t rows = ccall((:GetParamColRows, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if rows == 0 + return Array{Float64, 1}() + end + ptr = ccall((:GetParamCol, library), Ptr{Float64}, (Ptr{Nothing}, Cstring,), params, paramName) - return Base.unsafe_wrap(Array{Float64, 1}, ptr, rows, own=true) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + + return Base.unsafe_wrap(Array{Float64, 1}, ptr, rows, own=own) end -function GetParamRow(params::Ptr{Nothing}, paramName::String) +function GetParamRow(params::Ptr{Nothing}, + paramName::String, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptr::Ptr{Float64} local cols::Csize_t cols = ccall((:GetParamRowCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if cols == 0 + return Array{Float64, 1}(undef, 0) + end + ptr = ccall((:GetParamRow, library), Ptr{Float64}, (Ptr{Nothing}, Cstring,), params, paramName) - return Base.unsafe_wrap(Array{Float64, 1}, ptr, cols, own=true) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + + return Base.unsafe_wrap(Array{Float64, 1}, ptr, cols, own=own) end -function GetParamUCol(params::Ptr{Nothing}, paramName::String) +function GetParamUCol(params::Ptr{Nothing}, + paramName::String, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptr::Ptr{Csize_t} local rows::Csize_t rows = ccall((:GetParamUColRows, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if rows == 0 + return Array{Int, 1}() + end + ptr = ccall((:GetParamUCol, library), Ptr{Csize_t}, (Ptr{Nothing}, Cstring,), params, paramName) - m = Base.unsafe_wrap(Array{Csize_t, 1}, ptr, rows, own=true) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + + m = Base.unsafe_wrap(Array{Csize_t, 1}, ptr, rows, own=own) return convert(Array{Int, 1}, m .+ 1) end -function GetParamURow(params::Ptr{Nothing}, paramName::String) +function GetParamURow(params::Ptr{Nothing}, + paramName::String, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptr::Ptr{Csize_t} local cols::Csize_t cols = ccall((:GetParamURowCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if cols == 0 + return Array{Int, 1}(undef, 0) + end + ptr = ccall((:GetParamURow, library), Ptr{Csize_t}, (Ptr{Nothing}, Cstring,), params, paramName) - m = Base.unsafe_wrap(Array{Csize_t, 1}, ptr, cols, own=true) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptr) in juliaOwnedMemory) + + m = Base.unsafe_wrap(Array{Csize_t, 1}, ptr, cols, own=own) return convert(Array{Int, 1}, m .+ 1) end function GetParamMatWithInfo(params::Ptr{Nothing}, paramName::String, - pointsAsRows::Bool) + pointsAsRows::Bool, + juliaOwnedMemory::Set{Ptr{Nothing}}) local ptrBool::Ptr{Bool} local ptrData::Ptr{Float64} local rows::Csize_t @@ -377,20 +468,29 @@ function GetParamMatWithInfo(params::Ptr{Nothing}, Cstring,), params, paramName) cols = ccall((:GetParamMatWithInfoCols, library), Csize_t, (Ptr{Nothing}, Cstring,), params, paramName) + # Shortcut: if the vector is empty, return an empty matrix. + if rows == 0 && cols == 0 + return (Array{Bool, 1}(undef, 0), Array{Float64, 2}(undef, 0, 0)) + end + ptrBool = ccall((:GetParamMatWithInfoBoolPtr, library), Ptr{Bool}, (Ptr{Nothing}, Cstring,), params, paramName) ptrMem = ccall((:GetParamMatWithInfoPtr, library), Ptr{Float64}, (Ptr{Nothing}, Cstring,), params, paramName) + # Determine whether we should tell Julia to free this memory. If the memory + # originally came from Julia, then we won't own the wrapped object. + own = !(convert(Ptr{Nothing}, ptrMem) in juliaOwnedMemory) + types = Base.unsafe_wrap(Array{Bool, 1}, ptrBool, (rows), own=true) if pointsAsRows # In this case we have to transpose, unfortunately. - m = Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=true) + m = Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), own=own) return (types, m') else # Here no transpose is necessary. return (types, Base.unsafe_wrap(Array{Float64, 2}, ptr, (rows, cols), - own=true)) + own=own)) end end diff --git a/src/mlpack/bindings/julia/print_input_processing_impl.hpp b/src/mlpack/bindings/julia/print_input_processing_impl.hpp index 06969d0847..c605d7c727 100644 --- a/src/mlpack/bindings/julia/print_input_processing_impl.hpp +++ b/src/mlpack/bindings/julia/print_input_processing_impl.hpp @@ -104,7 +104,8 @@ void PrintInputProcessing( // Now print the SetParam call. std::cout << indent << "SetParam" << uChar << matTypeModifier << "(p, \"" - << d.name << "\", " << juliaName << extra << ")" << std::endl; + << d.name << "\", " << juliaName << extra << ", juliaOwnedMemory)" + << std::endl; if (!d.required) { @@ -180,8 +181,8 @@ void PrintInputProcessing( // // SetParam(p, "", convert(, )) std::cout << " SetParam(p, \"" << d.name << "\", convert(" - << GetJuliaType(d) << ", " << juliaName << "), points_are_rows)" - << std::endl; + << GetJuliaType(d) << ", " << juliaName << "), points_are_rows, " + << "juliaOwnedMemory)" << std::endl; } else { @@ -192,8 +193,8 @@ void PrintInputProcessing( // end std::cout << " if !ismissing(" << juliaName << ")" << std::endl; std::cout << " SetParam(p, \"" << d.name << "\", convert(" - << GetJuliaType(d) << ", " << juliaName << "), points_are_rows)" - << std::endl; + << GetJuliaType(d) << ", " << juliaName << "), points_are_rows, " + << "juliaOwnedMemory)" << std::endl; std::cout << " end" << std::endl; } } diff --git a/src/mlpack/bindings/julia/print_jl.cpp b/src/mlpack/bindings/julia/print_jl.cpp index 276c74090f..292b0850a5 100644 --- a/src/mlpack/bindings/julia/print_jl.cpp +++ b/src/mlpack/bindings/julia/print_jl.cpp @@ -257,6 +257,12 @@ void PrintJL(const string& bindingName, cout << " t = Timers()" << endl; cout << endl; + // Create a set where we will store the pointers associated with all memory + // that Julia owns. This is to prevent situations where we end up wrapping a + // result object and telling Julia to own that too---in that case, the GC will + // free the object twice! + cout << " juliaOwnedMemory = Set{Ptr{Nothing}}()" << endl; + // Handle each input argument's processing before calling mlpackMain(). cout << " # Process each input argument before calling mlpackMain()." << endl; diff --git a/src/mlpack/bindings/julia/print_output_processing_impl.hpp b/src/mlpack/bindings/julia/print_output_processing_impl.hpp index 1a3b7c0b9d..b3de0f688a 100644 --- a/src/mlpack/bindings/julia/print_output_processing_impl.hpp +++ b/src/mlpack/bindings/julia/print_output_processing_impl.hpp @@ -90,7 +90,7 @@ void PrintOutputProcessing( } std::cout << "GetParam" << uChar << matTypeSuffix << "(p, \"" << d.name - << "\"" << extra << ")"; + << "\"" << extra << ", juliaOwnedMemory)"; } /** @@ -120,7 +120,7 @@ void PrintOutputProcessing( const typename std::enable_if>::value>::type*) { - std::cout << "GetParamMatWithInfo(p, \"" << d.name << "\")"; + std::cout << "GetParamMatWithInfo(p, \"" << d.name << "\", juliaOwnedMemory)"; } } // namespace julia diff --git a/src/mlpack/bindings/julia/tests/runtests.jl b/src/mlpack/bindings/julia/tests/runtests.jl index 38ffef8ef5..fa95bfc7a5 100644 --- a/src/mlpack/bindings/julia/tests/runtests.jl +++ b/src/mlpack/bindings/julia/tests/runtests.jl @@ -216,6 +216,7 @@ end # Test a row vector input parameter. @testset "TestRow" begin x = rand(100) + oldX = copy(x) _, _, _, _, _, _, _, rowOut, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -224,7 +225,7 @@ end @test size(rowOut, 1) == 100 @test typeof(rowOut) == Array{Float64, 1} for i in 1:100 - @test rowOut[i] == 2 * x[i] + @test rowOut[i] == 2 * oldX[i] end end @@ -250,7 +251,7 @@ end x = rand(Float64, (10, 100)) # Dimension information. dims = [false, false, false, false, false, false, false, false, false, false] - z = x + z = copy(x) _, _, _, matrix_and_info_out, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -262,7 +263,7 @@ end for i in 1:100 for j in 1:10 - @test matrix_and_info_out[j, i] == 2.0 * z[j, i] + @test matrix_and_info_out[j, i] == 2.0 * x[j, i] end end end @@ -272,7 +273,7 @@ end x = rand(Float64, (100, 10)) # Dimension information. dims = [false, false, false, false, false, false, false, false, false, false] - z = x + z = copy(x) _, _, _, matrix_and_info_out, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -284,7 +285,7 @@ end for i in 1:100 for j in 1:10 - @test matrix_and_info_out[i, j] == 2.0 * z[i, j] + @test matrix_and_info_out[i, j] == 2.0 * x[i, j] end end end @@ -298,7 +299,7 @@ end rand(1:6, 100), rand(100))') dims = [false, true, false, true, true, false] - z = x + z = copy(x) _, _, _, matrix_and_info_out, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -310,10 +311,10 @@ end for i in 1:100 for j in [1, 3, 6] - @test matrix_and_info_out[j, i] == 2.0 * z[j, i] + @test matrix_and_info_out[j, i] == 2.0 * x[j, i] end for j in [2, 4, 5] - @test matrix_and_info_out[j, i] == z[j, i] + @test matrix_and_info_out[j, i] == x[j, i] end end end @@ -327,7 +328,7 @@ end rand(1:6, 100), rand(100)) dims = [false, true, false, true, true, false] - z = x + z = copy(x) _, _, _, matrix_and_info_out, _, _, _, _, _, _, _, _, _, _ = test_julia_binding(4.0, 12, "hello", @@ -339,10 +340,10 @@ end for i in 1:100 for j in [1, 3, 6] - @test matrix_and_info_out[i, j] == 2.0 * z[i, j] + @test matrix_and_info_out[i, j] == 2.0 * x[i, j] end for j in [2, 4, 5] - @test matrix_and_info_out[i, j] == z[i, j] + @test matrix_and_info_out[i, j] == x[i, j] end end end