Most CPP extension classes don’t think about serialization, and some require more initialization than the default contractor provides. The result is that on deserialization we end up with a partially initialized object, and undefined behavior after that.
Modified the serializer to emit __PHP_Unserializable_Class instead, and just the class name for the debugger case. Modified the deserializer to create a __PHP_Unserializable_Class, too, if we are given a string with a CPP extension instance in it, so we're safe on both sides. I do not raise a warning; the results are pretty clear, I think, and it's also unlikely that most such warnings could (or should) be corrected. Finally, I also removed the odd behavior of emitting "dummy" classes for Closure and Continuation. I found no use of these in any source tree. Closure had no implementation at all, and Continuation had an implementation that simply raised a fatal on any method call. Thus we can safely assume that even if someone was serializing one of these types, they were receiving an object that was useless to them, or would fatal. So I believe this transformation is safe.
Most of the values stored in TypedValue in ext_asio are always Cells.
Let's use Cell type alias to improve code readability, pass them by
reference where null is not allowed and use Cell-specific functions
from tv_helpers.h.
No emitted bytecode relies on Continuation being stored in local 0
anymore. Stop using local 0 for this purpose and compute offset
to the Continuation at JIT time. 16 bytes of memory freed.
At this point all locals of Continuation construction wrapper share the
same indices with their respective locals of Continuation body, which
should allow further optimizations.
When object support was first added to HHVM, a class named "Instance"
was introduced (deriving from ObjectData) to represent instances of user
defined classes. Since then, things have evolved and HPHPc and HPHPi have
been retired, and now there really is no needed to have ObjectData and
Instance be separate classes anymore.
This diff moves all of the functionality from the Instance class to the
ObjectData and removes the Instance class. In the process, I got rid of
a bunch of dead methods and fixed some indentation and other style issues.
Updates continuations to allow yielding of a key-value
pair from a generator. Adds bytecode instructions (PackContK,
ContKey) for using the new feature, and adds IR instructions
(ContUpdateIdx, ContIncKey) to help get it down to the metal
(in particular, ContIncKey attempts to keep the current use-cases
as fast as possible).
Continuation's m_received field is used to transfer values and
exceptions to the Continuation by send() and raise() methods. It has a
valid value only for a short duration of time, between
ContNext/ContSend/ContRaise and UnpackCont opcodes, when no other
operations could possibly occur. Let's free these 16 bytes of memory and
pass the value thru the VM stack.
To achieve this goal, ContEnter was modified to accept a value to be
transferred on the stack. The existence of the value on the stack is
then acknowledged by the UnpackCont opcode, which is the first opcode
executed after ContEnter enters the continuation body.
ContNext then becomes a trivial Null and is thus removed, ContSend just
teleports the value from local 0 to the stack (I will experiment in a
follow up diff with replacing ContSent by CGetL+UnsetL). ContRaise has
to update the label, but the eventual plan is to enter into unwinder
directly from the raise() method.
Save 8 bytes of m_args and its initialization for Continuations without
func_get_args() call (does not save real memory due to 16-byte alignment).
Store variable arguments in optional local.
Users of m_localsOffset used this offset to calculate pointer where
local variables starts. Since they are stored in the reverse order, the
equation was:
cont_ptr + m_localsOffset + num_locals - local_id - 1
We are already storing pointer to ActRec (m_arPtr), which is equal to:
cont_ptr + m_localsOffset + num_locals
This diffs kills m_localsOffset and uses m_arPtr to simplify
calculations. Continuation fields were rearranged so that
sizeof(c_Continuation) is reduced by 16 bytes.
Each closure instance carries its own set of static variables. Multiple
generators created using the same closure instance should share their
static variables.
This was not the case and static variables in closure generators behaved
like normal local variables. Let's fix it by referencing closure and
using its static locals.
Pointer to the closure is located in the local following original input
arguments. These are shifted by one, as Continuation occupies local 0.
This also frees 8 bytes from c_Continuation (or 16 if you count 16-byte
alignment).
m_obj in c_Continuation is redundant. It is only used at the time
Continuation is created. First, it is set in init() call. Few
instructions later, it is read so that $this in the Continuation's
ActRec can be populated.
Kill it.
After Variant::Variant(NullInit) was made explicit, passing
Variant::nullInit to a function expecting CVarRef, or Variant
would actually call Variant(0), with ensuing hilarity.
Make NullInit an enum class to prevent such issues.
Make sure m_received is always null at ContSend, ContRaise,
and ContNext, and this information to generate better code for these
bytecode instructions.
I'm committing my work on HphpArray in bite-sized pieces for easier review. This piece replaces calls to NEW with a factory method. There are many problems with operator new, starting with the fact that the allocator cannot communicate properly with the constructor.
The newly introduced factory method should return ArrayData but that causes many issues right now, so I left that step to a future diff.
I was learning from @jdelong and he said that you should use
double quotes for local includes and angle brackets for library
includes. I asked why our code was the way it was, and he said he wanted
to clean it up. I beat him to it :)
Conflicts:
hphp/runtime/base/server/admin_request_handler.cpp
hphp/runtime/vm/named_entity.h
ContinuationWaitHandle::start() was deprecated in favor of Awaitable
interface's getWaitHandle().
Remove ContinuationWaitHandle::start() from public API and rename it
internally to c_ContinuationWaitHandle::Create() that is called only from
Continuation's getWaitHandle(). Since it is guaranteed that the
Continuation does not have associated any wait handle, we can remove the
code that handles that case.
We store a bunch of data into an ActRec on the stack, then
call a php-level function that calls a C++ function to copy it into an
ActRec on the heap. This should hopefully be a little better.
This is a partial step towards merging the HPHP::VM namespace
up into its parent. To keep it reviewable/mergeable I'm not doing
everything at once here, but most of the code I've touched seems
improved. I've drawn an invisible line around the jit, Unit and
its cohort (Class, Func, PreClass, etc.); we'll get back to them
soon.
Generators should specify function names as the name instead of the
full name for consistency with Zend 5.5 (and more importantly because
this breaks PHPUnit), this was exposed by the backtrace removal of
file and line info
The only places where ReturnStatement is constructed are:
- onReturn(check_yield=true) -> not allowed in generator
- onReturn(check_yield=false) -> coming from transform_yield_break, right after creating hphp_continuation_done()
- MethodStatement, end of function call -> hphp_continuation_done() is created at end of generator in prepare_generator()
Emitter is emitting ContExit in ReturnStatements used in generators. As
can be seen from the analysis above, it's always preceded by emitting
ContDone from hphp_continuation_done(). Let's emit ContDone inside the
ReturnStatement directly and kill usage of hphp_continuation_done().
transform_yield_break() becomes a simple onReturn(check_yield=false), so
let's inline it into onYield and create ReturnStatement directly. After
this change, check_yield flag is always true and can be killed.
ContExit was also used after emitting a generator method in case the end
of method is still reachable. ContDone is added so that the generator is
properly closed. I believe this is never actually used, as MethodStatement
creates ReturnStatement at the end of method anyway.
Unhack the parser and introduce YieldExpression that emits the
equivalent set of opcodes that were emitted by bunch of
expressions/statements generated by parser before.
YieldExpression expects evaluation stack to contain just the value
being yielded, so {,List}AssignmentExpression need to evaluate RHS
first. The previous code had the same behavior.
This will let us consolidate continuation-related opcodes and make
them less tied with continuation objects.
We only used it to get the values of certain class constants,
and to define the ObjectStaticCallbacks for every class.
We can put the class constants directly into the class_map
(we should have done that before for perf reasons), and then
the only remaining use of ObjectStaticCallbacks is to proxy
the Class* for each builtin class. So just use the Class*
directly.
Once this is in, Im just a small step away from eliminating
make -C hphp/system - so Im leaving a lot of dead code here.
Its going to be easier to delete it en masse, rather than
try to pick and chose now.
While I'm looking at this stuff during debugging, get rid of some of the
stuff that's only compiled for the now-nonexistent hphpc build.
I'm aiming for this change to result in identical code after
preprocessing. There is some other stuff in these files that could go
(any method with ##const_assert(!hhvm)##) but getting rid of those is a
bit hairier; I tried, and it resulted in some weird, hard-to-repro
instability, and it doesn't seem worth sinking much time into at the
moment.
This change is mostly for FB internal organizational reasons.
Building is not effected beyond the fact that the target now
lands in hphp/hhvm/hhvm rather than src/hhvm/hhvm.
Mike Magruder