Make NativeDataInfo handlers optional
commit73a36d4bcc314a02c5588ad58b06b1e322b5d9c7
authorSara Golemon <sgolemon@fb.com>
Thu, 22 May 2014 02:26:16 +0000 (21 19:26 -0700)
committerJoelMarcey <joelm@fb.com>
Thu, 22 May 2014 18:15:45 +0000 (22 11:15 -0700)
tree5322c2d7e6287cd52a74ce0e08e3d1df8ac6d50d
parent56357d83a2ea560b2fc9eb1838a5045f457e7a75
Make NativeDataInfo handlers optional

Summary: In the case of SweepFunc, we can avoid adding
ourselves to the sweeplist entirely if there's no func.

I'm still allocating space for SweepNode atm, and I'd
like to fix that in a followup diff.

It probably means swapping the order of the NDI block
and the SweepNode block, otherwise Native::data ends up
looking like:

  template<class T>
  T* data(ObjectData *obj) {
    if (obj->nativeDataInfo()->sweep) {
      auto node = reinterpret_cast<SweepNode*>(obj) - 1;
      return reinterpret_cast<T*>(node) - 1;
    }
    return reinterpret_cast<T*>(obj) - 1;
  }

Which feels a lot more expensive for a relatively hot path.
If I do swap the order, then it simplifies down to just:

  template<class T>
  T* data(ObjectData *obj) {
    return reinterpret_cast<T*>(obj) - 1;
  }

and that's great for runtime, but Sweep gets weird since
SweepNode doesn't know where it's ObjectData* is relative
to itself.

I could make my own SweepNode struct which points at the
ObjectData* instead (which in turn knows how to map back
to the SweepNode), but that ends up looking like a weird
broken linked-list and takes longer to actually do the sweeps.

Or maybe it's fine to just leave those two extra pointer allocations.

Reviewed By: @jdelong

Differential Revision: D1291769
hphp/runtime/ext_zend_compat/hhvm/zend-object.cpp
hphp/runtime/vm/native-data.cpp
hphp/runtime/vm/native-data.h