|Summary:||Move currentThreadStackBase() to WTF|
|Product:||WebKit||Reporter:||Alp Toker <alp>|
|Severity:||Normal||CC:||ap, barraclough, mrowe|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Alp Toker 2008-11-26 22:43:04 PST
Comment 1 Alp Toker 2008-11-26 23:09:39 PST
Comment 2 Alexey Proskuryakov 2008-11-27 07:38:50 PST
> -static inline void* currentThreadStackBase() I'm wondering why this function was inline.
Comment 3 Alp Toker 2008-11-28 06:42:17 PST
(In reply to comment #2) > > -static inline void* currentThreadStackBase() > > I'm wondering why this function was inline. > I added CollectorUtilities.cpp to AllInOneFile.cpp just above Collector.cpp, guessing gcc will inline it if appropriate at -O3. If you're concerned it might affect performance, could you try running a perf test with this patch applied? I don't think inlining or not will affect the correctness of the result (this function is for finding the stack base, not stack pointer so it's fine as a function call).
Comment 4 Alexey Proskuryakov 2008-11-28 08:10:14 PST
I agree, I have no idea why it was inline.
Comment 5 Darin Adler 2008-12-05 10:06:03 PST
Comment on attachment 25554 [details] Share utility code for GC implementations I don't like the word "utilities" in a file name; it seems to be a word without meaning. I suggest calling this something like MachineStack.h. If the macro is going to be in a header file, then I think it should be changed into an inline function that takes a const void*, so we don't have to add a WTF_ prefix to it. Why include <stdlib.h>? I think all you need in this header is the type intptr_t, and that's not in <stdlib.h>.