From b6122ad3dc5ded2fe71dc3060eec4d1586962004 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Fri, 25 Jul 2014 14:25:22 +0200 Subject: [PATCH] Snapshot, this version has memcheck problems. --- arangod/Aql/ExecutionPlan.cpp | 95 +++++++++++++++++++++++------------ arangod/Aql/ExecutionPlan.h | 19 ++++--- lib/Basics/JsonHelper.h | 74 +++++++++++++++++---------- 3 files changed, 122 insertions(+), 66 deletions(-) diff --git a/arangod/Aql/ExecutionPlan.cpp b/arangod/Aql/ExecutionPlan.cpp index d73c88e185..e95480dc68 100644 --- a/arangod/Aql/ExecutionPlan.cpp +++ b/arangod/Aql/ExecutionPlan.cpp @@ -27,8 +27,6 @@ #include "Aql/ExecutionPlan.h" -#include - using namespace triagens::basics; using namespace triagens::aql; @@ -40,29 +38,28 @@ using namespace triagens::aql; /// @brief toJson, export an ExecutionPlan to JSON //////////////////////////////////////////////////////////////////////////////// -TRI_json_t* ExecutionPlan::toJson (TRI_memory_zone_t* zone) { +Json ExecutionPlan::toJson (TRI_memory_zone_t* zone) { Json json; try { json = Json(Json::Array,2) ("type", Json(getTypeString())); } catch (std::exception& e) { - return nullptr; + return json; } if (_dependencies.size() != 0) { - Json deps; try { - deps = Json(Json::List, _dependencies.size()); + Json deps(Json::List, _dependencies.size()); for (size_t i = 0; i < _dependencies.size(); i++) { deps(_dependencies[i]->toJson(zone)); } json("dependencies", deps); } catch (std::exception& e) { - return nullptr; + return Json(); // returns an empty one } } - return json.steal(); + return json; } //////////////////////////////////////////////////////////////////////////////// @@ -94,27 +91,35 @@ void ExecutionPlan::appendAsString (std::string& st, int indent) { st.push_back('>'); } +// ----------------------------------------------------------------------------- +// --SECTION-- methods of EnumerateCollectionPlan +// ----------------------------------------------------------------------------- + //////////////////////////////////////////////////////////////////////////////// /// @brief toJson, for EnumerateCollectionPlan //////////////////////////////////////////////////////////////////////////////// -TRI_json_t* EnumerateCollectionPlan::toJson (TRI_memory_zone_t* zone) { - auto ep = static_cast(this); - Json json(zone, ep->toJson(zone)); +Json EnumerateCollectionPlan::toJson (TRI_memory_zone_t* zone) { + Json json(ExecutionPlan::toJson(zone)); // call base class method if (json.isEmpty()) { - return nullptr; + return json; } // Now put info about vocbase and cid in there try { - json("vocbase", Json(_vocbase->_name)) - ("cid", JsonHelper::uint64String(zone, _cid)); + if (_vocbase == nullptr) { + json("vocbase", Json("")); + } + else { + json("vocbase", Json(_vocbase->_name)); + } + json("collection", Json(_collname)); } catch (std::exception& e) { - return nullptr; + return Json(); } // And return it: - return json.steal(); + return json; } //////////////////////////////////////////////////////////////////////////////// @@ -123,51 +128,63 @@ TRI_json_t* EnumerateCollectionPlan::toJson (TRI_memory_zone_t* zone) { using namespace triagens::basics; +using namespace std; + void testExecutionPlans () { + Json a(12); + Json b(Json::Array); + b("a",a); + std::cout << b.toString() << std::endl; + std::cout << a.toString() << std::endl; + std::cout << "Got here" << std::endl; ExecutionPlan* e = new ExecutionPlan(); ExecutionPlan* f = new ExecutionPlan(e); - std::string st; + string st; e->appendAsString(st, 0); - std::cout << "e as string:\n" << st << std::endl; + cout << "e as string:\n" << st << endl; st.clear(); f->appendAsString(st, 0); - std::cout << "f as string:\n" << st << std::endl; + cout << "f as string:\n" << st << endl; TRI_json_t* json = e->toJson(TRI_UNKNOWN_MEM_ZONE); if (json != nullptr) { - std::cout << "e as JSON:\n" << - JsonHelper::toString(json) << std::endl; + cout << "e as JSON:\n" << + JsonHelper::toString(json) << endl; } TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); json = f->toJson(TRI_UNKNOWN_MEM_ZONE); if (json != nullptr) { - std::cout << "f as JSON:\n" << - JsonHelper::toString(json) << std::endl; + cout << "f as JSON:\n" << + JsonHelper::toString(json) << endl; } TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); delete f; // should not leave a leak + auto ec = new EnumerateCollectionPlan(nullptr, "guck"); + Json jjj(ec->toJson()); + cout << jjj.toString() << endl; + json = Json(12); - std::cout << JsonHelper::toString(json) << std::endl; + cout << JsonHelper::toString(json) << endl; TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); json = Json(true); - std::cout << JsonHelper::toString(json) << std::endl; + cout << JsonHelper::toString(json) << endl; TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); json = Json(Json::Null); - std::cout << JsonHelper::toString(json) << std::endl; + cout << JsonHelper::toString(json) << endl; TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); json = Json(Json::String); - std::cout << JsonHelper::toString(json) << std::endl; + cout << JsonHelper::toString(json) << endl; TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); json = Json(Json::List); - std::cout << JsonHelper::toString(json) << std::endl; + cout << JsonHelper::toString(json) << endl; TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); json = Json(Json::Array); - std::cout << JsonHelper::toString(json) << std::endl; + cout << JsonHelper::toString(json) << endl; TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); json = Json(Json::Array, 10) @@ -182,7 +199,7 @@ void testExecutionPlans () { ("myarray", Json(Json::Array, 2) ("a",Json("hallo")) ("b",Json(13))); - std::cout << JsonHelper::toString(json) << std::endl; + cout << JsonHelper::toString(json) << endl; TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); Json j(Json::Array); @@ -193,14 +210,26 @@ void testExecutionPlans () { ("d", Json(Json::Array) ("x", Json(12)) ("y", Json(true))); - std::cout << j.toString() << std::endl; + cout << j.toString() << endl; - std::cout << j.get("a").toString() << std::endl; + // We expect to see exactly two copies here: + Json jjjj = j.copy(); // create an explicit copy + Json jj(12); + + cout << "Before assignment" << jj.toString() << endl; + jj = j; // this steals the pointer from j + + cout << "Before copy" << jj.toString() << endl; + jj = j.copy(); // this does a copy, but both are now NOFREE + + cout << j.get("a").toString() << endl; + cout << jjjj.toString(); + cout << jj.toString(); Json k = j.get("c"); Json l = k.at(2); - std::cout << l.toString() << std::endl; + cout << l.toString() << endl; } // Local Variables: diff --git a/arangod/Aql/ExecutionPlan.h b/arangod/Aql/ExecutionPlan.h index b11fff3d87..df9456bb65 100644 --- a/arangod/Aql/ExecutionPlan.h +++ b/arangod/Aql/ExecutionPlan.h @@ -31,6 +31,7 @@ #include #include +#include #include #include @@ -160,10 +161,11 @@ namespace triagens { } //////////////////////////////////////////////////////////////////////////////// -/// @brief export to JSON +/// @brief export to JSON, returns an AUTOFREE Json object //////////////////////////////////////////////////////////////////////////////// - virtual TRI_json_t* toJson (TRI_memory_zone_t* zone); + virtual triagens::basics::Json toJson ( + TRI_memory_zone_t* zone = TRI_UNKNOWN_MEM_ZONE); //////////////////////////////////////////////////////////////////////////////// /// @brief convert to a string, basically for debugging purposes @@ -195,8 +197,10 @@ namespace triagens { /// @brief constructor with just a collection ID //////////////////////////////////////////////////////////////////////////////// - EnumerateCollectionPlan (TRI_vocbase_t* vocbase, TRI_voc_cid_t cid) - : ExecutionPlan(), _vocbase(vocbase), _cid(cid) { + public: + + EnumerateCollectionPlan (TRI_vocbase_t* vocbase, std::string collname) + : ExecutionPlan(), _vocbase(vocbase), _collname(collname) { } //////////////////////////////////////////////////////////////////////////////// @@ -219,20 +223,21 @@ namespace triagens { /// @brief export to JSON //////////////////////////////////////////////////////////////////////////////// - virtual TRI_json_t* toJson (TRI_memory_zone_t* zone); + virtual triagens::basics::Json toJson ( + TRI_memory_zone_t* zone = TRI_UNKNOWN_MEM_ZONE); // ----------------------------------------------------------------------------- // --SECTION-- private variables // ----------------------------------------------------------------------------- //////////////////////////////////////////////////////////////////////////////// -/// @brief our dependent nodes +/// @brief we need to know the database and the collection //////////////////////////////////////////////////////////////////////////////// private: TRI_vocbase_t* _vocbase; - TRI_voc_cid_t _cid; + std::string _collname; }; } // namespace triagens::aql diff --git a/lib/Basics/JsonHelper.h b/lib/Basics/JsonHelper.h index 31e13199bb..bd5b361a7f 100644 --- a/lib/Basics/JsonHelper.h +++ b/lib/Basics/JsonHelper.h @@ -481,6 +481,19 @@ namespace triagens { Json (Json const& j) : _zone(j._zone), _json(j.steal()), _autofree(j._autofree) { + std::cout << "Hallo copy constructor" << std::endl; + } + +//////////////////////////////////////////////////////////////////////////////// +/// @brief move constructor, note that in the AUTOFREE case this steals +/// the structure from j to allow returning Json objects by value without +/// copying the whole structure. +//////////////////////////////////////////////////////////////////////////////// + + Json (Json const&& j) + : _zone(j._zone), _json(j.steal()), _autofree(j._autofree) { + std::cout << "Hallo, move constructor at work for " + << this->toString() << std::endl; } //////////////////////////////////////////////////////////////////////////////// @@ -488,7 +501,9 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// ~Json () throw() { + std::cout << "Destructor for " << this->toString() << std::endl; if (_json != nullptr && _autofree == AUTOFREE) { + std::cout << "Actually TRI_FreeJson called" << std::endl; TRI_FreeJson(_zone, _json); } } @@ -528,43 +543,50 @@ namespace triagens { } //////////////////////////////////////////////////////////////////////////////// -/// @brief assignment operator, note that this does an actual recursive copy, -/// if AUTOFREE is set for j. If you need steal semantics, for example -/// when assigning a temporary object, then use "assign" +/// @brief assignment operator, note that, as the copy constructor, this +/// has steal semantics, which avoids deep copies in situations that +/// people will use. If you need an actual copy, use the copy method. //////////////////////////////////////////////////////////////////////////////// Json& operator= (Json const& j) { - if (_json != nullptr && _autofree == AUTOFREE) { - TRI_FreeJson(_zone, _json); - } - _zone = j._zone; - _autofree = j._autofree; - if (_autofree == AUTOFREE) { - std::cout << "ATTENTION: recursive JSON copy performed!!!" << std::endl; - _json = TRI_CopyJson(_zone, j._json); - } - else { - _json = j._json; - } - return *this; - } - -//////////////////////////////////////////////////////////////////////////////// -/// @brief assignment operator, note that this does never produce an actual -/// recursive copy of j, even if AUTOFREE is set for j. That is, you get -/// steal semantics, this is for example useful when assigning a -/// temporary object. -//////////////////////////////////////////////////////////////////////////////// - - void assign (Json const& j) { + std::cout << "= called" << std::endl; if (_json != nullptr && _autofree == AUTOFREE) { TRI_FreeJson(_zone, _json); } _zone = j._zone; _autofree = j._autofree; _json = j.steal(); + return *this; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief move assignment operator, this has steal semantics. +//////////////////////////////////////////////////////////////////////////////// + + Json& operator= (Json const&& j) { + std::cout << "= move called" << std::endl; + if (_json != nullptr && _autofree == AUTOFREE) { + TRI_FreeJson(_zone, _json); + } + _zone = j._zone; + _autofree = j._autofree; + _json = j.steal(); + return *this; + } + +//////////////////////////////////////////////////////////////////////////////// +/// @brief copy recursively, even if NOFREE is set! +//////////////////////////////////////////////////////////////////////////////// + + Json copy () { + Json c; + std::cout << "ATTENTION: recursive JSON copy performed!!!" << std::endl; + c._zone = _zone; + c._json = TRI_CopyJson(_zone, _json); + c._autofree = _autofree; + return c; + } + //////////////////////////////////////////////////////////////////////////////// /// @brief set an attribute value in an array, an exception is thrown /// if *this is not a Json array. Note that you can call this with