[Lldb-commits] [PATCH] D12936: Groundwork for better tracking of renderscript allocations and scripts.
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 17 09:44:38 PDT 2015
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
A few suggestions to move things over into RenderScriptRuntime.cpp and to possibly use std::unique_ptr, but they are just suggestions.
================
Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:258-259
@@ -263,1 +257,4 @@
+
+ typedef std::shared_ptr<ScriptDetails> ScriptDetailsSP;
+ typedef std::shared_ptr<AllocationDetails> AllocationDetailsSP;
----------------
Are you actually handing out shared pointers to anyone? If you are just using std::shared_ptr so you can put things into the vector, you can probably just use std::unique_ptr.
================
Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:298
@@ +297,3 @@
+ // then a new Script detail object will be created for this address and returned.
+ ScriptDetailsSP LookUpScript(lldb::addr_t address, bool create);
+
----------------
If you only need memory management you could say that "m_scripts" owns the objects (if they start using std::unique_ptr) and hand out a raw pointer here. Not sure on your memory management needs though.
================
Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:304
@@ +303,3 @@
+ // detail object will be created for this address and returned.
+ AllocationDetailsSP LookUpAllocation (lldb::addr_t address, bool create);
+};
----------------
Ditto above comment.
================
Comment at: source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h:307-417
@@ -295,2 +306,113 @@
+
+// The empirical_type adds a basic level of validation to arbitrary data
+// allowing us to track if data has been discovered and stored or not.
+// An empirical_type will be marked as valid only if it has been explicitly assigned to.
+template <typename type_t>
+class empirical_type
+{
+ public:
+ // Ctor. Contents is invalid when constructed.
+ empirical_type()
+ : valid(false)
+ {}
+
+ // Return true and copy contents to out if valid, else return false.
+ bool get(type_t& out) const
+ {
+ if (valid)
+ out = data;
+ return valid;
+ }
+
+ // Return a pointer to the contents or nullptr if it was not valid.
+ const type_t* get() const
+ {
+ return valid ? &data : nullptr;
+ }
+
+ // Assign data explicitly.
+ void set(const type_t in)
+ {
+ data = in;
+ valid = true;
+ }
+
+ // Mark contents as invalid.
+ void invalidate()
+ {
+ valid = false;
+ }
+
+ // Returns true if this type contains valid data.
+ bool isValid() const
+ {
+ return valid;
+ }
+
+ // Assignment operator.
+ empirical_type<type_t>& operator = (const type_t in)
+ {
+ set(in);
+ return *this;
+ }
+
+ // Dereference operator returns contents.
+ // Warning: Will assert if not valid so use only when you know data is valid.
+ const type_t& operator * () const
+ {
+ assert(valid);
+ return data;
+ }
+
+ protected:
+ bool valid;
+ type_t data;
+};
+
+// The ScriptDetails class collects data associated with a single script instance.
+struct RenderScriptRuntime::ScriptDetails
+{
+ enum ScriptType
+ {
+ eScript,
+ eScriptC
+ };
+
+ // The derived type of the script.
+ empirical_type<ScriptType> type;
+ // The name of the original source file.
+ empirical_type<std::string> resName;
+ // Path to script .so file on the device.
+ empirical_type<std::string> scriptDyLib;
+ // Directory where kernel objects are cached on device.
+ empirical_type<std::string> cacheDir;
+ // Pointer to the context which owns this script.
+ empirical_type<lldb::addr_t> context;
+ // Pointer to the script object itself.
+ empirical_type<lldb::addr_t> script;
+};
+
+// This AllocationDetails class collects data associated with a single
+// allocation instance.
+struct RenderScriptRuntime::AllocationDetails
+{
+ enum DataType
+ {
+ eInt,
+ };
+
+ enum Dimension
+ {
+ e1d,
+ e2d,
+ e3d,
+ eCubeMap,
+ };
+
+ empirical_type<DataType> type;
+ empirical_type<Dimension> dimension;
+ empirical_type<lldb::addr_t> address;
+ empirical_type<lldb::addr_t> dataPtr;
+ empirical_type<lldb::addr_t> context;
};
----------------
Does anyone really need to see these objects outside of RenderScriptRuntime.cpp? If not, move these into RenderScriptRuntime.cpp.
Repository:
rL LLVM
http://reviews.llvm.org/D12936
More information about the lldb-commits
mailing list