[threading] #diff 3-3 final!

Summary: added some more tests and fixed ExecutionContext's fiberExit() call site. Unmarshaling back timezone so I can write a unit test. This would be my last change before some www perf numbers. Code complete! If it's too slow, i'll work on some copy-on-write FiberSharedVariant wrappers. Test Plan: test/test TestCodeRunStatic TestFiber DiffCamp Revision: 131826 Reviewed By: iproctor CC: hphp-diffs@lists, iproctor Tasks: #141384: add multi-threading support to PHP Revert Plan: OK
huichen · Jul 12, 2010 · de1f844 · de1f844
1 parent 6ee06ba
commit de1f844
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 16 deletions.
diff --git a/doc/index.php b/doc/index.php
@@ -12,9 +12,9 @@
         'Server Status' => 'server.status',
         'Server Statistics' => 'server.stats',
       ),
-      'PHP Compatibility' => array(
-        'Inconsistencies' => 'inconsistencies',
-        'Compiled vs. Interpreter' => 'inconsistencies.hphpi',
+      'Inconsistencies' => array(
+        'PHP vs. HipHop' => 'inconsistencies',
+        'HipHop Compiler vs. Interpreter' => 'inconsistencies.hphpi',
       ),
       'New Features' => array(
         'New functions' => 'extension.new_functions',

diff --git a/doc/threading b/doc/threading
@@ -74,14 +74,17 @@ specified.
 
 $default_strategy can be,
 
-(1) GLOBAL_STATE_IGNORE
+(1) GLOBAL_STATE_IGNORE (default)
 
 In this case, new thread's modifications of global states will be thrown away.
 
-(2) GLOBAL_STATE_OVERWRITE (default)
+(2) GLOBAL_STATE_OVERWRITE
 
 In this case, new thread's modifications of global states will entirely
-replace main thread's.
+replace main thread's. Do so ONLY when you are sure main thread's global states
+can be lost during this overwrite. Otherwise, please use GLOBAL_STATE_IGNORE
+and use function output parameters to pass back global states that need to
+be copied back to main thread.
 
 (3) GLOBAL_STATE_SKIP
 

diff --git a/src/idl/function.idl.php b/src/idl/function.idl.php
@@ -32,7 +32,7 @@
 
 f('end_user_func_async', Variant,
   array('handle' => Object,
-        'default_strategy' => array(Int32, 'k_GLOBAL_STATE_OVERWRITE'),
+        'default_strategy' => array(Int32, 'k_GLOBAL_STATE_IGNORE'),
         'additional_strategies' => array(Variant, 'null')));
 
 f('forward_static_call_array', Variant,

diff --git a/src/runtime/base/execution_context.cpp b/src/runtime/base/execution_context.cpp
@@ -106,6 +106,17 @@ void ExecutionContext::fiberExit(FiberLocal *src, FiberReferenceMap &refMap) {
       }
     }
   }
+
+  // isNull() is really a sub-optimal way of telling "who's changed".
+  if (m_timezone.isNull()) {
+    m_timezone = ec->m_timezone.fiberCopy();
+  }
+  if (m_timezoneDefault.isNull()) {
+    m_timezoneDefault = ec->m_timezoneDefault.fiberCopy();
+  }
+  if (m_argSeparatorOutput.isNull()) {
+    m_argSeparatorOutput = ec->m_argSeparatorOutput.fiberCopy();
+  }
 }
 
 ///////////////////////////////////////////////////////////////////////////////

diff --git a/src/runtime/base/fiber_async_func.cpp b/src/runtime/base/fiber_async_func.cpp
@@ -87,10 +87,6 @@ class FiberJob : public Synchronizable {
     if (m_unmarshaled_function) {
       Lock lock(m_thread->m_mutex);
       if (m_thread->m_reqId == m_reqId) {
-        ExecutionContext *context = g_context.get();
-        if (m_context && context) {
-          m_context->fiberExit(context, m_refMap);
-        }
         DELETE(Variant)(m_unmarshaled_function);
         DELETE(Variant)(m_unmarshaled_params);
         m_unmarshaled_function = NULL;
@@ -120,6 +116,7 @@ class FiberJob : public Synchronizable {
       ExecutionContext *context = g_context.get();
       if (context && m_context) {
         context->fiberInit(m_context, m_refMap);
+        m_context = context; // switching role
       }
       m_function = m_function.fiberMarshal(m_refMap);
       m_params = m_params.fiberMarshal(m_refMap);
@@ -171,6 +168,12 @@ class FiberJob : public Synchronizable {
       while (!m_done) wait();
     }
 
+    ExecutionContext *context = g_context.get();
+    if (context && m_context) {
+      context->fiberExit(m_context, m_refMap);
+      m_context = NULL;
+    }
+
     fiber_unmarshal_global_state(get_global_variables(), m_global_variables,
                                  m_refMap, default_strategy, resolver);
 
@@ -219,7 +222,7 @@ class FiberJob : public Synchronizable {
   FiberAsyncFuncData *m_thread;
 
   // holding references to them, so we can later restore their states safely
-  ExecutionContext *m_context; // main thread's
+  ExecutionContext *m_context; // peer thread's
   Variant *m_unmarshaled_function;
   Variant *m_unmarshaled_params;
   GlobalVariables *m_unmarshaled_global_variables;

diff --git a/src/runtime/ext/ext_function.cpp b/src/runtime/ext/ext_function.cpp
@@ -112,7 +112,7 @@ Object f_call_user_func_async(int _argc, CVarRef function,
 }
 
 Variant f_end_user_func_async(CObjRef handle,
-                              int default_strategy /* = k_GLOBAL_STATE_OVERWRITE */,
+                              int default_strategy /* = k_GLOBAL_STATE_IGNORE */,
                               CVarRef additional_strategies /* = null */) {
   return FiberAsyncFunc::Result(handle,
                                 (FiberAsyncFunc::Strategy)default_strategy,

diff --git a/src/runtime/ext/ext_function.h b/src/runtime/ext/ext_function.h
@@ -32,7 +32,7 @@ Object f_call_user_func_array_async(CVarRef function, CArrRef params);
 Object f_call_user_func_async(int _argc, CVarRef function, CArrRef _argv = null_array);
 Variant f_forward_static_call_array(CVarRef function, CArrRef params);
 Variant f_forward_static_call(int _argc, CVarRef function, CArrRef _argv = null_array);
-Variant f_end_user_func_async(CObjRef handle, int default_strategy = k_GLOBAL_STATE_OVERWRITE, CVarRef additional_strategies = null);
+Variant f_end_user_func_async(CObjRef handle, int default_strategy = k_GLOBAL_STATE_IGNORE, CVarRef additional_strategies = null);
 String f_create_function(CStrRef args, CStrRef code);
 
 ///////////////////////////////////////////////////////////////////////////////

diff --git a/src/runtime/ext/profile/extprofile_function.h b/src/runtime/ext/profile/extprofile_function.h
@@ -60,7 +60,7 @@ inline Object x_call_user_func_async(int _argc, CVarRef function, CArrRef _argv
   return f_call_user_func_async(_argc, function, _argv);
 }
 
-inline Variant x_end_user_func_async(CObjRef handle, int default_strategy = k_GLOBAL_STATE_OVERWRITE, CVarRef additional_strategies = null) {
+inline Variant x_end_user_func_async(CObjRef handle, int default_strategy = k_GLOBAL_STATE_IGNORE, CVarRef additional_strategies = null) {
   FUNCTION_INJECTION_BUILTIN(end_user_func_async);
   return f_end_user_func_async(handle, default_strategy, additional_strategies);
 }

diff --git a/src/system/function.inc b/src/system/function.inc
@@ -6,7 +6,7 @@
 "call_user_func", T(Variant), S(0), "function", T(Variant), NULL, S(0), NULL, S(16), 
 "call_user_func_array_async", T(Object), S(0), "function", T(Variant), NULL, S(0), "params", T(Array), NULL, S(0), NULL, S(0), 
 "call_user_func_async", T(Object), S(0), "function", T(Variant), NULL, S(0), NULL, S(16), 
-"end_user_func_async", T(Variant), S(0), "handle", T(Object), NULL, S(0), "default_strategy", T(Int32), "k_GLOBAL_STATE_OVERWRITE", S(0), "additional_strategies", T(Variant), "null", S(0), NULL, S(0), 
+"end_user_func_async", T(Variant), S(0), "handle", T(Object), NULL, S(0), "default_strategy", T(Int32), "k_GLOBAL_STATE_IGNORE", S(0), "additional_strategies", T(Variant), "null", S(0), NULL, S(0), 
 "forward_static_call_array", T(Variant), S(0), "function", T(Variant), NULL, S(0), "params", T(Array), NULL, S(0), NULL, S(0), 
 "forward_static_call", T(Variant), S(0), "function", T(Variant), NULL, S(0), NULL, S(16), 
 "create_function", T(String), S(0), "args", T(String), NULL, S(0), "code", T(String), NULL, S(0), NULL, S(0), 

diff --git a/src/test/test_code_run.cpp b/src/test/test_code_run.cpp
@@ -11888,6 +11888,26 @@ bool TestCodeRun::TestFiber() {
 
         "int(456)\n"
        );
+  MVCRO("<?php "
+        "function fiber() { global $foo; $foo = 456;}"
+        "$foo = 123;"
+        "end_user_func_async(call_user_func_async('fiber'),"
+        " GLOBAL_STATE_IGNORE);"
+        "var_dump($foo);",
+
+        "int(123)\n"
+       );
+  MVCRO("<?php "
+        "function fiber() { global $foo; $foo = 456;}"
+        "$foo = 123;"
+        "end_user_func_async(call_user_func_async('fiber'),"
+        " GLOBAL_STATE_OVERWRITE,"
+        " array(GLOBAL_SYMBOL_GLOBAL_VARIABLE =>"
+        "       array('foo' => GLOBAL_STATE_IGNORE)));"
+        "var_dump($foo);",
+
+        "int(123)\n"
+       );
 
   // test dynamic globals
   MVCRO("<?php "
@@ -11909,6 +11929,16 @@ bool TestCodeRun::TestFiber() {
         "int(125)\n"
        );
 
+  // test execution context
+  MVCRO("<?php "
+        "function fiber() { date_default_timezone_set('America/New_York');}"
+        "$task = call_user_func_async('fiber');"
+        "end_user_func_async($task);"
+        "var_dump(date_default_timezone_get());",
+
+        "string(16) \"America/New_York\"\n"
+       );
+
   return true;
 }