Summary: | [GTK][EFL] Enable SamplingProfiler | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
Component: | WebKitGTK | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bugs-noreply, cgarcia, clopez, gyuyoung.kim, jh718.park, joepeck, mcatanzaro, ossy, saam | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 154033 | ||||||
Bug Blocks: | 153466 | ||||||
Attachments: |
|
Description
Yusuke Suzuki
2016-01-28 21:28:57 PST
The GTK+ port currently aims to support x64, x86, ARM, ARM64 and MIPS on Linux with the GNU libc (GLIBC). Support for running WebKitGTK+ on other architectures or other operating systems / C libraries like BSD, MacOS or Android is something that we don't plan to actively support for the moment. But if there is someone interested on this, and they submit reasonable patches we will try to help with the review. (In reply to comment #1) > The GTK+ port currently aims to support x64, x86, ARM, ARM64 and MIPS on > Linux with the GNU libc (GLIBC). > > Support for running WebKitGTK+ on other architectures or other operating > systems / C libraries like BSD, MacOS or Android is something that we don't > plan to actively support for the moment. But if there is someone interested > on this, and they submit reasonable patches we will try to help with the > review. OK, so now, the current sampling profiler completely meets the GTK port's requirements. So, for GTK port, we would like to make #if (OS(DARWIN) || OS(WINDOWS) || (USE(PTHREADS) && defined(__GLIBC__))) && ENABLE(JIT) to #if (OS(DARWIN) || OS(WINDOWS) || PLATFORM(GTK)) && ENABLE(JIT) And once sampling profiler is enabled, we will drop legacy profiler. How about EFL port? (In reply to comment #2) > #if (OS(DARWIN) || OS(WINDOWS) || (USE(PTHREADS) && defined(__GLIBC__))) && > ENABLE(JIT) > > to > > #if (OS(DARWIN) || OS(WINDOWS) || PLATFORM(GTK)) && ENABLE(JIT) This is ENABLE_SAMPLING_PROFILER flag. I think this needs to be added as a build option in WebKitFeatures.cmake. We would make it public in OptionsGTK.cmake, so it can be disabled on platforms where it does not work. (In reply to comment #4) > I think this needs to be added as a build option in WebKitFeatures.cmake. We > would make it public in OptionsGTK.cmake, so it can be disabled on platforms > where it does not work. Sounds nice :) Created attachment 270816 [details]
Patch
Comment on attachment 270816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270816&action=review > Source/cmake/OptionsGTK.cmake:167 > +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SAMPLING_PROFILER PUBLIC ON) I'm not sure this line is necessary. This is a list of private options that differ from the list in WebKitFeatures.cmake. The option you've added is public (shows up in the configuration UI) and the default value is the same. I think you can just remove it. Is there a reason you had in mind to make this option public? Comment on attachment 270816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270816&action=review > Source/WTF/wtf/Platform.h:802 > * sampling profiler is enabled if WebKit uses pthreads and glibc. */ This comment needs to be updated. >> Source/cmake/OptionsGTK.cmake:167 >> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SAMPLING_PROFILER PUBLIC ON) > > I'm not sure this line is necessary. This is a list of private options that differ from the list in WebKitFeatures.cmake. The option you've added is public (shows up in the configuration UI) and the default value is the same. I think you can just remove it. Is there a reason you had in mind to make this option public? Yeah, it's added in the wrong place in OptionsGTK.cmake, good catch Martin! This should be moved to the list of public options instead. I suggested making it public because it does not work on some platforms, so there should be some way to turn it off. > Source/cmake/WebKitFeatures.cmake:175 > + WEBKIT_OPTION_DEFINE(ENABLE_SAMPLING_PROFILER "Toggle sampling profier support" PRIVATE ON) profier -> profiler Comment on attachment 270816 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270816&action=review Thanks for your reviews :) >>> Source/cmake/OptionsGTK.cmake:167 >>> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SAMPLING_PROFILER PUBLIC ON) >> >> I'm not sure this line is necessary. This is a list of private options that differ from the list in WebKitFeatures.cmake. The option you've added is public (shows up in the configuration UI) and the default value is the same. I think you can just remove it. Is there a reason you had in mind to make this option public? > > Yeah, it's added in the wrong place in OptionsGTK.cmake, good catch Martin! This should be moved to the list of public options instead. I suggested making it public because it does not work on some platforms, so there should be some way to turn it off. Oops! Thanks :D I've fixed; moving this flag to PUBLIC list. >> Source/cmake/WebKitFeatures.cmake:175 >> + WEBKIT_OPTION_DEFINE(ENABLE_SAMPLING_PROFILER "Toggle sampling profier support" PRIVATE ON) > > profier -> profiler Thanks! Fixed. Committed r196245: <http://trac.webkit.org/changeset/196245> *** Bug 153466 has been marked as a duplicate of this bug. *** |