PolicyArray initial review

PolicyArray splits the ArrayData implementation in two parts. ArrayShell implements the baroque ArrayData implementation in terms of a much smaller statically-bound core that concerns itself exclusively with the storage strategy for the array. This way the two aspects can be worked on separately, and different stores can be easily plugged into the given ArrayShell.

To give a better perspective, the featured SimpleArrayStore has about 340 lines all told (including solid documentation), whereas ArrayShell has some 1100 lines. The shell can be reused with other stores of unbounded sophistication. The store needs to implement only 22 primitives, some of which are trivial. Basically a new store implementation saves 1100 of difficult-to-get-right lines of code right off the bat, and only needs to focus on implementing 22 well-defined primitives.

Things to watch for when reviewing:

- Is the store API small/expressive enough? What should be added/removed?

- Are various forms of duplication present? If so, how could code be factored better?

- Any subtle change in semantics from HphpArray and friends that should be minded?

Known issues:

* Currently ArrayShell is defined as:

  class ArrayShell : public ArrayData, private SimpleArrayStore { ... };

when in fact it should be:

  template <class StorePolicy>
  class ArrayShell : public ArrayData, private StorePolicy { ... };

Extenuating circumstances related to allocator design prevent use of templates at this point. I'll work on these in parallel with the review.

* growNoResize is almost always prefaced by a test-and-grow. This sequence should be encoded in a function.

* The growth policy is spread all over the place in a subtle form of duplication.

* The current store design makes almost no effort to be particularly efficient.
Esse commit está contido em:
aalexandre
2013-04-17 20:47:25 -07:00
commit de Sara Golemon
commit 9559dcf879
12 arquivos alterados com 1744 adições e 86 exclusões
+6
Ver Arquivo
@@ -26,6 +26,7 @@
#include <runtime/base/shared/shared_map.h>
#include <util/exception.h>
#include <tbb/concurrent_hash_map.h>
#include <runtime/base/array/policy_array.h>
namespace HPHP {
///////////////////////////////////////////////////////////////////////////////
@@ -115,6 +116,11 @@ void ArrayData::release() {
that->release();
return;
}
if (isArrayShell()) {
auto that = static_cast<ArrayShell*>(this);
that->release();
return;
}
assert(m_kind == kNameValueTableWrapper);
// NameValueTableWrapper: nop.
}