Make crate thread-safe (by removing set_global_codegen_config) (#43)

* Add thread-safe BinaryenModuleOptimizeWithSettings

* Derive Default trait on CodegenConfig (which sets -O0)

* Change optimize() to take CodegenConfig

* Add thread-safe BinaryenModuleRunPassesWithSettings

* Change run_optimization_passes() to take CodegenConfig

* Remove set_global_codegen_config

Author: axic

binaryen-sys/Shim.cpp

@@ -17,7 +17,7 @@
 using namespace wasm;
 using namespace std;
 
-// NOTE: this is a copy from binaryen-c.cpp
+// NOTE: this is based on BinaryenModuleRead from binaryen-c.cpp
 extern "C" BinaryenModuleRef BinaryenModuleSafeRead(const char* input, size_t inputSize) {
     auto* wasm = new Module;
     vector<char> buffer(input, input + inputSize);
@@ -56,3 +56,36 @@ extern "C" void BinaryenShimDisposeBinaryenModuleAllocateAndWriteResult(
         free(result.sourceMap);
     }
 }
+
+// NOTE: this is based on BinaryenModuleOptimizer from binaryen-c.cpp
+// Main benefit is being thread safe.
+extern "C" void BinaryenModuleOptimizeWithSettings(
+    BinaryenModuleRef module, int shrinkLevel, int optimizeLevel, int debugInfo
+) {
+  Module* wasm = (Module*)module;
+  PassRunner passRunner(wasm);
+  passRunner.options = PassOptions::getWithDefaultOptimizationOptions();
+  passRunner.options.shrinkLevel = shrinkLevel;
+  passRunner.options.optimizeLevel = optimizeLevel;
+  passRunner.options.debugInfo = debugInfo != 0;
+  passRunner.addDefaultOptimizationPasses();
+  passRunner.run();
+}
+
+// NOTE: this is based on BinaryenModuleRunPasses from binaryen-c.cpp
+// Main benefit is being thread safe.
+extern "C" void BinaryenModuleRunPassesWithSettings(
+    BinaryenModuleRef module, const char** passes, BinaryenIndex numPasses,
+    int shrinkLevel, int optimizeLevel, int debugInfo
+) {
+  Module* wasm = (Module*)module;
+  PassRunner passRunner(wasm);
+  passRunner.options = PassOptions::getWithDefaultOptimizationOptions();
+  passRunner.options.shrinkLevel = shrinkLevel;
+  passRunner.options.optimizeLevel = optimizeLevel;
+  passRunner.options.debugInfo = debugInfo != 0;
+  for (BinaryenIndex i = 0; i < numPasses; i++) {
+    passRunner.add(passes[i]);
+  }
+  passRunner.run();
+}

binaryen-sys/wrapper.h

@@ -14,6 +14,15 @@ void BinaryenShimDisposeBinaryenModuleAllocateAndWriteResult(
     BinaryenModuleAllocateAndWriteResult result
 );
 
+void BinaryenModuleOptimizeWithSettings(
+    BinaryenModuleRef module, int shrinkLevel, int optimizeLevel, int debugInfo
+);
+
+void BinaryenModuleRunPassesWithSettings(
+    BinaryenModuleRef module, const char** passes, BinaryenIndex numPasses,
+    int shrinkLevel, int optimizeLevel, int debugInfo
+);
+
 #ifdef __cplusplus
 }
 #endif

examples/wasm_opt.rs

@@ -97,8 +97,7 @@ fn main() {
         }
     };
     let mut module = read_module(&args.input_path);
-    binaryen::set_global_codegen_config(&args.codegen_config);
-    module.optimize();
+    module.optimize(&args.codegen_config);
 
     let optimized_wasm = module.write();
     write_module(&args.output_path, &optimized_wasm);

src/lib.rs

@@ -11,15 +11,13 @@ pub use binaryen_sys as ffi;
 use std::rc::Rc;
 use std::os::raw::c_char;
 use std::{ptr, slice};
-use std::sync::{Once, Mutex};
 use std::ffi::CString;
 use std::str::FromStr;
 
 pub mod tools;
 
 /// Codegen configuration.
-/// 
-/// Use `set_global_codegen_config`.
+#[derive(Default)]
 pub struct CodegenConfig {
     /// 0, 1, 2 correspond to -O0, -Os, -Oz
     pub shrink_level: u32,
@@ -29,38 +27,6 @@ pub struct CodegenConfig {
     pub debug_info: bool,
 }
 
-/// Set the global code generation configuration.
-/// 
-/// This can be used to set parameters before running `optimize` function. 
-/// However, this can influence behavior of running binaryen passes in general (for example, 
-/// `auto_drop` is implemented via a pass).
-pub fn set_global_codegen_config(codegen_config: &CodegenConfig) {
-    static mut MUTEX: Option<Mutex<()>> = None;
-    static INIT: Once = Once::new();
-
-    // Initialize the mutex only once.
-    INIT.call_once(|| {
-        unsafe {
-            // This is safe since we are in `call_once`, and it will execute this closure only once.
-            // If the second invocation happens before the closure returned then the invocation will be blocked until 
-            // the closure returns.
-            MUTEX = Some(Mutex::new(()));
-        }
-    });
-
-    unsafe {
-        let _guard = MUTEX
-            .as_ref()
-            .expect("should be initialized in call_once block above")
-            .lock()
-            .unwrap();
-
-        ffi::BinaryenSetOptimizeLevel(codegen_config.optimization_level as i32);
-        ffi::BinaryenSetShrinkLevel(codegen_config.shrink_level as i32);
-        ffi::BinaryenSetDebugInfo(codegen_config.debug_info as i32);
-    }
-}
-
 fn is_valid_pass(pass: &str) -> bool {
     ffi::passes::OptimizationPass::from_str(pass).is_ok()
 }
@@ -112,18 +78,22 @@ impl Module {
     }
 
     /// Run the standard optimization passes on the module.
-    /// 
-    /// It will take into account code generation configuration set by `set_global_codegen_config`.
-    pub fn optimize(&mut self) {
-        unsafe { ffi::BinaryenModuleOptimize(self.inner.raw) }
+    pub fn optimize(&mut self, codegen_config: &CodegenConfig) {
+        unsafe {
+            ffi::BinaryenModuleOptimizeWithSettings(
+                self.inner.raw,
+                codegen_config.shrink_level as i32,
+                codegen_config.optimization_level as i32,
+                codegen_config.debug_info as i32
+            )
+        }
     }
 
     /// Run a specified set of optimization passes on the module.
-    ///
-    /// This WILL NOT take into account code generation configuration set by `set_global_codegen_config`.
     pub fn run_optimization_passes<B: AsRef<str>, I: IntoIterator<Item = B>>(
         &mut self,
         passes: I,
+        codegen_config: &CodegenConfig
     ) -> Result<(), ()> {
         let mut cstr_vec: Vec<_> = vec![];
 
@@ -141,7 +111,16 @@ impl Module {
             .map(|pass| pass.as_ptr())
             .collect();
 
-        unsafe { ffi::BinaryenModuleRunPasses(self.inner.raw, ptr_vec.as_mut_ptr(), ptr_vec.len() as u32) };
+        unsafe {
+            ffi::BinaryenModuleRunPassesWithSettings(
+                self.inner.raw,
+                ptr_vec.as_mut_ptr(),
+                ptr_vec.len() as u32,
+                codegen_config.shrink_level as i32,
+                codegen_config.optimization_level as i32,
+                codegen_config.debug_info as i32
+            )
+        };
         Ok(())
     }
 
@@ -217,15 +196,15 @@ mod tests {
 
         assert!(module.is_valid());
 
-        module.run_optimization_passes(&["vacuum", "untee"]).expect("passes succeeded");
+        module.run_optimization_passes(&["vacuum", "untee"], &CodegenConfig::default()).expect("passes succeeded");
 
         assert!(module.is_valid());
     }
 
     #[test]
     fn test_invalid_optimization_passes() {
         let mut module = Module::new();
-        assert!(module.run_optimization_passes(&["invalid"]).is_err());
+        assert!(module.run_optimization_passes(&["invalid"], &CodegenConfig::default()).is_err());
     }
 
     #[test]
@@ -243,7 +222,7 @@ mod tests {
 
         let mut module = Module::read(&input).unwrap();
         assert!(module.is_valid());
-        module.optimize();
+        module.optimize(&CodegenConfig::default());
         assert!(module.is_valid());
         assert_eq!(module.write(), expected);
     }