Clean up ScriptGroup2 implementation
b/19944127
- Adjust ref counting
- Fix value size type in closure creation
Change-Id: I1179d34aa67f845578740e71cc2da4f82419f251
diff --git a/cpu_ref/rsCpuScriptGroup2.cpp b/cpu_ref/rsCpuScriptGroup2.cpp
index 3a50221..a202a42 100644
--- a/cpu_ref/rsCpuScriptGroup2.cpp
+++ b/cpu_ref/rsCpuScriptGroup2.cpp
@@ -121,7 +121,7 @@
if (it != argDeps.end()) {
const auto& args = (*it).second;
for (const auto &p1 : *args) {
- if (p1.second->get() != nullptr) {
+ if (p1.second.get() != nullptr) {
return true;
}
}
@@ -188,11 +188,11 @@
for (Batch* batch : mBatches) {
delete batch;
}
+ delete mExecutable;
// TODO: move this dlclose into ~ScriptExecutable().
if (mScriptObj != nullptr) {
dlclose(mScriptObj);
}
- delete mExecutable;
}
namespace {
@@ -379,7 +379,7 @@
}
mExecutable = ScriptExecutable::createFromSharedObject(
- nullptr, // RS context. Unused.
+ getCpuRefImpl()->getContext(),
mScriptObj);
#endif // RS_COMPATIBILITY_LIB
@@ -408,8 +408,6 @@
continue;
}
rsAssert(p.first != nullptr);
- ALOGV("Evaluating closure %p, setting field %p (Script %p, slot: %d)",
- closure, p.first, p.first->mScript, p.first->mSlot);
Script* script = p.first->mScript;
const RsdCpuScriptImpl *cpuScript =
(const RsdCpuScriptImpl*)script->mHal.drv;
diff --git a/rs.spec b/rs.spec
index 8054faa..6524f6f 100644
--- a/rs.spec
+++ b/rs.spec
@@ -308,7 +308,7 @@
param RsAllocation returnValue
param RsScriptFieldID * fieldIDs
param uintptr_t * values
- param size_t * sizes
+ param int * sizes
param RsClosure * depClosures
param RsScriptFieldID * depFieldIDs
ret RsClosure
@@ -320,7 +320,7 @@
param const void * params
param const RsScriptFieldID * fieldIDs
param const uintptr_t * values
- param const size_t * sizes
+ param const int * sizes
ret RsClosure
}
diff --git a/rsClosure.cpp b/rsClosure.cpp
index 8fb12b8..4b57be4 100644
--- a/rsClosure.cpp
+++ b/rsClosure.cpp
@@ -12,7 +12,7 @@
RsAllocation returnValue,
RsScriptFieldID* fieldIDs, size_t fieldIDs_length,
uintptr_t* values, size_t values_length,
- size_t* sizes, size_t sizes_length,
+ int* sizes, size_t sizes_length,
RsClosure* depClosures, size_t depClosures_length,
RsScriptFieldID* depFieldIDs,
size_t depFieldIDs_length) {
@@ -20,31 +20,29 @@
sizes_length == depClosures_length &&
depClosures_length == depFieldIDs_length);
- return (RsClosure)(new Closure(
+ Closure* c = new Closure(
context, (const ScriptKernelID*)kernelID, (Allocation*)returnValue,
fieldIDs_length, (const ScriptFieldID**)fieldIDs, (const void**)values,
sizes, (const Closure**)depClosures,
- (const ScriptFieldID**)depFieldIDs));
+ (const ScriptFieldID**)depFieldIDs);
+ c->incUserRef();
+ return static_cast<RsClosure>(c);
}
RsClosure rsi_InvokeClosureCreate(Context* context, RsScriptInvokeID invokeID,
const void* params, const size_t paramLength,
const RsScriptFieldID* fieldIDs, const size_t fieldIDs_length,
const uintptr_t* values, const size_t values_length,
- const size_t* sizes, const size_t sizes_length) {
+ const int* sizes, const size_t sizes_length) {
rsAssert(fieldIDs_length == values_length && values_length == sizes_length);
- return (RsClosure)(new Closure(
+ Closure* c = new Closure(
context, (const ScriptInvokeID*)invokeID, params, paramLength,
fieldIDs_length, (const ScriptFieldID**)fieldIDs, (const void**)values,
- sizes));
+ sizes);
+ c->incUserRef();
+ return static_cast<RsClosure>(c);
}
-#if 0
-void rsi_ClosureEval(Context* rsc, RsClosure closure) {
- ((Closure*)closure)->eval();
-}
-#endif
-
void rsi_ClosureSetArg(Context* rsc, RsClosure closure, uint32_t index,
uintptr_t value, size_t size) {
((Closure*)closure)->setArg(index, (const void*)value, size);
@@ -63,7 +61,7 @@
const int numValues,
const ScriptFieldID** fieldIDs,
const void** values,
- const size_t* sizes,
+ const int* sizes,
const Closure** depClosures,
const ScriptFieldID** depFieldIDs) :
ObjectBase(context), mContext(context), mFunctionID((IDBase*)kernelID),
@@ -80,16 +78,6 @@
for (; i < (size_t)numValues; i++) {
rsAssert(fieldIDs[i] != nullptr);
mGlobals[fieldIDs[i]] = make_pair(values[i], sizes[i]);
- ALOGV("Creating closure %p, binding field %p (Script %p, slot: %d)",
- this, fieldIDs[i], fieldIDs[i]->mScript, fieldIDs[i]->mSlot);
- }
-
- size_t j = mNumArg;
- for (const auto& p : mGlobals) {
- rsAssert(p.first == fieldIDs[j]);
- rsAssert(p.second.first == values[j]);
- rsAssert(p.second.second == sizes[j]);
- j++;
}
for (i = 0; i < mNumArg; i++) {
@@ -97,11 +85,10 @@
if (dep != nullptr) {
auto mapping = mArgDeps[dep];
if (mapping == nullptr) {
- mapping = new Map<int, const ObjectBaseRef<ScriptFieldID>*>();
+ mapping = new Map<int, ObjectBaseRef<ScriptFieldID>>();
mArgDeps[dep] = mapping;
}
- (*mapping)[i] = new ObjectBaseRef<ScriptFieldID>(
- const_cast<ScriptFieldID*>(depFieldIDs[i]));
+ (*mapping)[i].set(const_cast<ScriptFieldID*>(depFieldIDs[i]));
}
}
@@ -110,14 +97,12 @@
if (dep != nullptr) {
auto mapping = mGlobalDeps[dep];
if (mapping == nullptr) {
- mapping = new Map<const ObjectBaseRef<ScriptFieldID>*,
- const ObjectBaseRef<ScriptFieldID>*>();
+ mapping = new Map<const ScriptFieldID*,
+ ObjectBaseRef<ScriptFieldID>>();
mGlobalDeps[dep] = mapping;
}
- (*mapping)[new ObjectBaseRef<ScriptFieldID>(
- const_cast<ScriptFieldID*>(fieldIDs[i]))] =
- new ObjectBaseRef<ScriptFieldID>(
- const_cast<ScriptFieldID*>(depFieldIDs[i]));
+ fieldIDs[i]->incSysRef();
+ (*mapping)[fieldIDs[i]].set(const_cast<ScriptFieldID*>(depFieldIDs[i]));
}
}
}
@@ -125,8 +110,9 @@
Closure::Closure(Context* context, const ScriptInvokeID* invokeID,
const void* params, const size_t paramLength,
const size_t numValues, const ScriptFieldID** fieldIDs,
- const void** values, const size_t* sizes) :
+ const void** values, const int* sizes) :
ObjectBase(context), mContext(context), mFunctionID((IDBase*)invokeID), mIsKernel(false),
+ mArgs(nullptr), mNumArg(0),
mReturnValue(nullptr), mParams(params), mParamLength(paramLength) {
for (size_t i = 0; i < numValues; i++) {
mGlobals[fieldIDs[i]] = make_pair(values[i], sizes[i]);
@@ -136,17 +122,13 @@
Closure::~Closure() {
for (const auto& p : mArgDeps) {
auto map = p.second;
- for (const auto& p1 : *map) {
- delete p1.second;
- }
delete p.second;
}
for (const auto& p : mGlobalDeps) {
auto map = p.second;
for (const auto& p1 : *map) {
- delete p1.first;
- delete p1.second;
+ p1.first->decSysRef();
}
delete p.second;
}
@@ -159,7 +141,7 @@
}
void Closure::setGlobal(const ScriptFieldID* fieldID, const void* value,
- const size_t size) {
+ const int size) {
mGlobals[fieldID] = make_pair(value, size);
}
diff --git a/rsClosure.h b/rsClosure.h
index d9e41a5..ad44821 100644
--- a/rsClosure.h
+++ b/rsClosure.h
@@ -25,7 +25,7 @@
const int numValues,
const ScriptFieldID** fieldIDs,
const void** values, // Allocations or primitive (numeric) types
- const size_t* sizes, // size for data type. -1 indicates an allocation.
+ const int* sizes, // size for data type. -1 indicates an allocation.
const Closure** depClosures,
const ScriptFieldID** depFieldIDs);
Closure(Context* context,
@@ -35,7 +35,7 @@
const size_t numValues,
const ScriptFieldID** fieldIDs,
const void** values, // Allocations or primitive (numeric) types
- const size_t* sizes); // size for data type. -1 indicates an allocation.
+ const int* sizes); // size for data type. -1 indicates an allocation.
virtual ~Closure();
@@ -45,7 +45,7 @@
void setArg(const uint32_t index, const void* value, const size_t size);
void setGlobal(const ScriptFieldID* fieldID, const void* value,
- const size_t size);
+ const int size);
Context* mContext;
@@ -62,18 +62,18 @@
size_t mNumArg;
// A global could be allocation or any primitive data type.
- Map<const ScriptFieldID*, Pair<const void*, size_t>> mGlobals;
+ Map<const ScriptFieldID*, Pair<const void*, int>> mGlobals;
Allocation* mReturnValue;
// All the other closures which this closure depends on for one of its
// arguments, and the fields which it depends on.
- Map<const Closure*, Map<int, const ObjectBaseRef<ScriptFieldID>*>*> mArgDeps;
+ Map<const Closure*, Map<int, ObjectBaseRef<ScriptFieldID>>*> mArgDeps;
// All the other closures that this closure depends on for one of its fields,
// and the fields that it depends on.
- Map<const Closure*, Map<const ObjectBaseRef<ScriptFieldID>*,
- const ObjectBaseRef<ScriptFieldID>*>*> mGlobalDeps;
+ Map<const Closure*, Map<const ScriptFieldID*,
+ ObjectBaseRef<ScriptFieldID>>*> mGlobalDeps;
const void* mParams;
const size_t mParamLength;
diff --git a/rsScriptGroup2.cpp b/rsScriptGroup2.cpp
index df97c9e..32590f6 100644
--- a/rsScriptGroup2.cpp
+++ b/rsScriptGroup2.cpp
@@ -5,6 +5,12 @@
namespace android {
namespace renderscript {
+ScriptGroup2::~ScriptGroup2() {
+ if (mRSC->mHal.funcs.scriptgroup.destroy) {
+ mRSC->mHal.funcs.scriptgroup.destroy(mRSC, this);
+ }
+}
+
void ScriptGroup2::execute(Context* rsc) {
if (rsc->mHal.funcs.scriptgroup.execute) {
rsc->mHal.funcs.scriptgroup.execute(rsc, this);
@@ -21,6 +27,8 @@
rsc->mHal.funcs.scriptgroup.init(rsc, group);
}
+ group->incUserRef();
+
return group;
}
diff --git a/rsScriptGroup2.h b/rsScriptGroup2.h
index 92a0f10..bc89df8 100644
--- a/rsScriptGroup2.h
+++ b/rsScriptGroup2.h
@@ -13,18 +13,11 @@
class ScriptGroup2 : public ScriptGroupBase {
public:
- /*
- TODO:
- Inputs and outputs are set and retrieved in Java runtime.
- They are opaque in the C++ runtime.
- For better compiler optimizations (of a script group), we need to include
- input and output information in the C++ runtime.
- */
ScriptGroup2(Context* rsc, const char* cacheDir, Closure** closures,
size_t numClosures) :
ScriptGroupBase(rsc), mClosures(closures, closures + numClosures),
mCacheDir(cacheDir) {}
- virtual ~ScriptGroup2() {}
+ virtual ~ScriptGroup2();
virtual SG_API_Version getApiVersion() const { return SG_V2; }
virtual void execute(Context* rsc);