tpunt / phactor

An implementation of the Actor model for PHP
BSD 3-Clause "New" or "Revised" License
61 stars 5 forks source link

Change the initially allocated VM stack size for each actor #10

Closed tpunt closed 6 years ago

tpunt commented 6 years ago

Currently, the VM will allocate a new stack of 262144 bytes (256kb). This makes actors very expensive in terms of memory usage. The default stack allocation size should be lowered, since the stack sizes for actors will generally be much lower than seen in a typical PHP application.

I have a patch for this, but it causes heap corruption issues for some reason:

diff --git a/src/classes/actor.c b/src/classes/actor.c
index 4a410c5257..4b2ccb70a4 100644
--- a/src/classes/actor.c
+++ b/src/classes/actor.c
@@ -311,7 +311,16 @@ zend_object* phactor_actor_ctor(zend_class_entry *entry)

     ph_mcontext_init(&actor_internal->context.mc, process_message_handler);

-    zend_vm_stack_init();
+    zend_vm_stack page = (zend_vm_stack)emalloc(PH_VM_STACK_SIZE);
+    page->top = ZEND_VM_STACK_ELEMENTS(page);
+    page->end = (zval*)((char*)page + PH_VM_STACK_SIZE);
+    page->prev = NULL;
+
+    EG(vm_stack) = page;
+    EG(vm_stack)->top++;
+    EG(vm_stack_top) = EG(vm_stack)->top;
+    EG(vm_stack_end) = EG(vm_stack)->end;
+
     ph_vmcontext_get(&actor_internal->context.vmc);

     /*
diff --git a/src/ph_context.h b/src/ph_context.h
index d494c577da..249b269d45 100644
--- a/src/ph_context.h
+++ b/src/ph_context.h
@@ -53,6 +53,8 @@ typedef struct _ph_context_t {
     ph_vmcontext_t vmc;
 } ph_context_t;

+#define PH_VM_STACK_SIZE 256 // starting size of PHP's VM stack
+
 #ifdef PH_FIXED_STACK_SIZE
 # define STACK_ALIGNMENT 16 // Ensure 16 byte stack alignment (for OS X)

Strangely enough, 256 bytes causes a few test failures (such as the actor-cleanup-on-shutdown.phpt test) with a heap corruption issue, and 512 bytes causes the context-switching-basics3.phpt test to fail with a segfault. Using 64, 128, 1024, 2048, etc, bytes seems to work fine. This needs some investigation before it gets changed.

tpunt commented 6 years ago

The heap corruption bug has been fixed in 008f0c7de9d1bda832aaeea2e398608dc6bd61e6 and the default VM stack size of an actor has been changed to 256 bytes in 81e0bec14cb9d9984d678d85d3ef3f8c74b07712.