Closed Bug 433337 Opened 12 years ago Closed 12 years ago
.c on Windows
Splitting it caused a slight performance regression on windows. The file should be reunified there, and member functions should be declared static rather than extern.
Adding this to the 1.9.1 triage queue.
Priority: -- → P1
The patch makes helper functions for js_Interpret either static and compiled as a part of jsinterp.cpp or external depending on the value of the JS_LONE_INTERPRET macro. By default JS_LONE_INTERPRET is set to 0 (static case) with MSVC and to otherwise.
Comment on attachment 327773 [details] [diff] [review] v1 >-#ifdef js_invoke_c__ Could we rename this jsinvoke_cpp___ to match the convention for header file idempotent-include guard macros? No extra underscores (js_) and the suffix is .cpp now, plus the triple _ at the end, IOW. >+#if JS_LONE_INTERPRET >+# define JS_INTERPRET_HELPER(declaration) extern declaration; >+# define JS_INTERPRET_HELPER_MODIFIER >+#else >+# define JS_INTERPRET_HELPER(declaration) >+# define JS_INTERPRET_HELPER_MODIFIER static >+#endif > >-extern JSClass * >-js_IsActiveWithOrBlock(JSContext *cx, JSObject *obj, int stackDepth); >+JS_INTERPRET_HELPER(jsval * >+js_AllocRawStack(JSContext *cx, uintN nslots, void **markp)) These are overparenthesized and unsightly, and possibly confusing to ctags-ish tools. Would it be simpler and shorter to just #ifdef the .h file externs? /be
The new version of the patch addresses the suggestions from the comment 3. In particular, now jsinterp.h simply ifdefs the declarations of the interpreter helpers.
Comment on attachment 328200 [details] [diff] [review] v2 >+/* >+ * JS_LONE_INTERPRET indicates that the compiler should see just the code for >+ * the js_Interpret function when compiling jsinterp.cpp. The rest of the code >+ * from the file should be visible only when compiling jsinvoke.cpp. It allows >+ * to optimize selectively js_Interpret when the granularity of the "It allows platform builds to optimize ...." Looks good otherwise, thanks. /be
Attachment #328200 - Flags: review?(brendan) → review+
The new version of the patch fixes the comments: * from the file should be visible only when compiling jsinvoke.cpp. It allows - * to optimize selectively js_Interpret when the granularity of the - * optimizations with the given compiler is a compilation unit. + * platform builds to optimize selectively js_Interpret when the granularity + * of the optimizations with the given compiler is a compilation unit. *
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.