153638 2016-01-28 21:28:57 -0800 [GTK][EFL] Enable SamplingProfiler 2016-02-09 09:06:01 -0800 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 154033 153466 1 ysuzuki ysuzuki bugs-noreply cgarcia clopez gyuyoung.kim jh718.park joepeck mcatanzaro ossy saam oldest_to_newest 1160008 0 ysuzuki 2016-01-28 21:28:57 -0800 We would like to enable Sampling Profiler in GTK and EFL. Currently, it's usable in GTK and EFL with - using Pthreads - using GLIBC - x64, x86, arm64, arm, mips arch So we would like to discuss - what CPU arch GTK and EFL should support - what standard library GTK and EFL should support Maybe, I think BSD and Android Bionic are needed. Android Bionic may be used in android-jsc which is used in Facebooks' React Native for Android[1]. [1]: https://github.com/facebook/android-jsc 1161253 1 clopez 2016-02-02 04:54:15 -0800 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. 1161258 2 ysuzuki 2016-02-02 06:27:30 -0800 (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? 1161259 3 ysuzuki 2016-02-02 06:28:14 -0800 (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. 1161308 4 mcatanzaro 2016-02-02 10:30:23 -0800 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. 1161331 5 ysuzuki 2016-02-02 11:24:28 -0800 (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 :) 1162726 6 270816 ysuzuki 2016-02-07 07:31:37 -0800 Created attachment 270816 Patch 1162773 7 270816 mrobinson 2016-02-07 13:34:29 -0800 Comment on attachment 270816 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? 1162785 8 270816 mcatanzaro 2016-02-07 14:26:27 -0800 Comment on attachment 270816 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 1162801 9 270816 ysuzuki 2016-02-07 17:31:01 -0800 Comment on attachment 270816 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. 1162802 10 ysuzuki 2016-02-07 17:34:14 -0800 Committed r196245: <http://trac.webkit.org/changeset/196245> 1162804 11 ysuzuki 2016-02-07 17:36:39 -0800 *** Bug 153466 has been marked as a duplicate of this bug. *** 270816 2016-02-07 07:31:37 -0800 2016-02-07 14:26:27 -0800 Patch bug-153638-20160208003113.patch text/plain 4748 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMTk2MjM4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGE4NTQ3ZmQxYzU3ZWZmNWQyNzA4ZjQ0 Y2E4MGQ2MGIzOTNjOTY5YWIuLjZkMTllMGY4YzM5MTZhOTUwODA4MWFjZDI3NWIxNzRiZTY2YmJi N2UgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTIgQEAKKzIwMTYtMDItMDcgIFl1c3VrZSBTdXp1a2kgIDx1dGF0 YW5lLnRlYUBnbWFpbC5jb20+CisKKyAgICAgICAgW0dUS11bRUZMXSBFbmFibGUgU2FtcGxpbmdQ cm9maWxlcgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTUzNjM4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg KiB3dGYvUGxhdGZvcm0uaDoKKwogMjAxNi0wMi0wNiAgRGFyaW4gQWRsZXIgIDxkYXJpbkBhcHBs ZS5jb20+CiAKICAgICAgICAgRmluaXNoIGF1ZGl0aW5nIGNhbGwgc2l0ZXMgb2YgdXBwZXIoKSBh bmQgbG93ZXIoKSwgZWxpbWluYXRlIG1hbnksIGFuZCByZW5hbWUgdGhlIGZ1bmN0aW9ucwpkaWZm IC0tZ2l0IGEvU291cmNlL1dURi93dGYvUGxhdGZvcm0uaCBiL1NvdXJjZS9XVEYvd3RmL1BsYXRm b3JtLmgKaW5kZXggODYxNTM5ZGRjZDEzMTc3MWQyMDM5NjdhN2RlNWExMjZlMjU0MTlmYy4uMTgx YjA5NDA3ZjJlMmY0ZDBjNWQzZjlkMDYzYTBhMDM4NmIyYmE5MiAxMDA2NDQKLS0tIGEvU291cmNl L1dURi93dGYvUGxhdGZvcm0uaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9QbGF0Zm9ybS5oCkBAIC04 MDAsNyArODAwLDcgQEAKICAqIEpTQyB0byBtZWFzdXJlIHdoZXJlIHRpbWUgaXMgc3BlbnQgaW5z aWRlIGEgSmF2YVNjcmlwdCBwcm9ncmFtLgogICogSW4gY29uZmlndXJhdGlvbnMgb3RoZXIgdGhh biBXaW5kb3dzIGFuZCBEYXJ3aW4sIGJlY2F1c2UgbGF5b3V0IG9mIG1jb250ZXh0X3QgZGVwZW5k cyBvbiBzdGFuZGFyZCBsaWJyYXJpZXMgKGxpa2UgZ2xpYmMpLAogICogc2FtcGxpbmcgcHJvZmls ZXIgaXMgZW5hYmxlZCBpZiBXZWJLaXQgdXNlcyBwdGhyZWFkcyBhbmQgZ2xpYmMuICovCi0jaWYg KE9TKERBUldJTikgfHwgT1MoV0lORE9XUykgfHwgKFVTRShQVEhSRUFEUykgJiYgZGVmaW5lZChf X0dMSUJDX18pKSkgJiYgRU5BQkxFKEpJVCkKKyNpZiAoT1MoREFSV0lOKSB8fCBPUyhXSU5ET1dT KSB8fCBQTEFURk9STShHVEspIHx8IFBMQVRGT1JNKEVGTCkpICYmIEVOQUJMRShKSVQpCiAjZGVm aW5lIEVOQUJMRV9TQU1QTElOR19QUk9GSUxFUiAxCiAjZWxzZQogI2RlZmluZSBFTkFCTEVfU0FN UExJTkdfUFJPRklMRVIgMApkaWZmIC0tZ2l0IGEvU291cmNlL2NtYWtlL09wdGlvbnNFZmwuY21h a2UgYi9Tb3VyY2UvY21ha2UvT3B0aW9uc0VmbC5jbWFrZQppbmRleCAwNmUxZTJmY2E3YzJhODBj NjkwM2I2ZTAwOGU0NzYwYjZjN2U2NzMyLi42OWVmODdmZmMzZjhiZWQyNTg1Y2M0Y2U1MTQ2NzM2 MzkxNjU3ZTgyIDEwMDY0NAotLS0gYS9Tb3VyY2UvY21ha2UvT3B0aW9uc0VmbC5jbWFrZQorKysg Yi9Tb3VyY2UvY21ha2UvT3B0aW9uc0VmbC5jbWFrZQpAQCAtMTIxLDYgKzEyMSw3IEBAIFdFQktJ VF9PUFRJT05fREVGQVVMVF9QT1JUX1ZBTFVFKEVOQUJMRV9QT0lOVEVSX0xPQ0sgUFJJVkFURSBP RkYpCiBXRUJLSVRfT1BUSU9OX0RFRkFVTFRfUE9SVF9WQUxVRShFTkFCTEVfUFVCTElDX1NVRkZJ WF9MSVNUIFBSSVZBVEUgT04pCiBXRUJLSVRfT1BUSU9OX0RFRkFVTFRfUE9SVF9WQUxVRShFTkFC TEVfUkVTT0xVVElPTl9NRURJQV9RVUVSWSBQVUJMSUMgT04pCiBXRUJLSVRfT1BUSU9OX0RFRkFV TFRfUE9SVF9WQUxVRShFTkFCTEVfUkVRVUVTVF9BTklNQVRJT05fRlJBTUUgUFVCTElDIE9OKQor V0VCS0lUX09QVElPTl9ERUZBVUxUX1BPUlRfVkFMVUUoRU5BQkxFX1NBTVBMSU5HX1BST0ZJTEVS IFBVQkxJQyBPTikKIFdFQktJVF9PUFRJT05fREVGQVVMVF9QT1JUX1ZBTFVFKEVOQUJMRV9TRUND T01QX0ZJTFRFUlMgUFVCTElDIE9GRikKIFdFQktJVF9PUFRJT05fREVGQVVMVF9QT1JUX1ZBTFVF KEVOQUJMRV9TSEFET1dfRE9NIFBSSVZBVEUgT0ZGKQogV0VCS0lUX09QVElPTl9ERUZBVUxUX1BP UlRfVkFMVUUoRU5BQkxFX1NQRUVDSF9TWU5USEVTSVMgUFVCTElDIE9OKQpkaWZmIC0tZ2l0IGEv U291cmNlL2NtYWtlL09wdGlvbnNHVEsuY21ha2UgYi9Tb3VyY2UvY21ha2UvT3B0aW9uc0dUSy5j bWFrZQppbmRleCBkZDE4MDEzNGI0YTg0ZjUzZWY3NTU1YWRhNGVlYjliNGE0MjJlYWFlLi42Nzk5 OTA2NGJmYmQ3YTg1ZWVkOTAxYjJlMTZkMmUzODZmZjQ0MDg2IDEwMDY0NAotLS0gYS9Tb3VyY2Uv Y21ha2UvT3B0aW9uc0dUSy5jbWFrZQorKysgYi9Tb3VyY2UvY21ha2UvT3B0aW9uc0dUSy5jbWFr ZQpAQCAtMTY0LDYgKzE2NCw3IEBAIFdFQktJVF9PUFRJT05fREVGQVVMVF9QT1JUX1ZBTFVFKEVO QUJMRV9NSFRNTCBQUklWQVRFIE9OKQogV0VCS0lUX09QVElPTl9ERUZBVUxUX1BPUlRfVkFMVUUo RU5BQkxFX05PVElGSUNBVElPTlMgUFJJVkFURSBPTikKIFdFQktJVF9PUFRJT05fREVGQVVMVF9Q T1JUX1ZBTFVFKEVOQUJMRV9QVUJMSUNfU1VGRklYX0xJU1QgUFJJVkFURSBPTikKIFdFQktJVF9P UFRJT05fREVGQVVMVF9QT1JUX1ZBTFVFKEVOQUJMRV9SRVFVRVNUX0FOSU1BVElPTl9GUkFNRSBQ UklWQVRFIE9OKQorV0VCS0lUX09QVElPTl9ERUZBVUxUX1BPUlRfVkFMVUUoRU5BQkxFX1NBTVBM SU5HX1BST0ZJTEVSIFBVQkxJQyBPTikKIFdFQktJVF9PUFRJT05fREVGQVVMVF9QT1JUX1ZBTFVF KEVOQUJMRV9TTU9PVEhfU0NST0xMSU5HIFBSSVZBVEUgT04pCiBXRUJLSVRfT1BUSU9OX0RFRkFV TFRfUE9SVF9WQUxVRShFTkFCTEVfVVNFUlNFTEVDVF9BTEwgUFJJVkFURSBPTikKIFdFQktJVF9P UFRJT05fREVGQVVMVF9QT1JUX1ZBTFVFKEVOQUJMRV9VU0VSX01FU1NBR0VfSEFORExFUlMgUFJJ VkFURSBPTikKZGlmZiAtLWdpdCBhL1NvdXJjZS9jbWFrZS9XZWJLaXRGZWF0dXJlcy5jbWFrZSBi L1NvdXJjZS9jbWFrZS9XZWJLaXRGZWF0dXJlcy5jbWFrZQppbmRleCA4OTE0NDI1NTU0NGU4Njcx OTAyYTExOGU3MmVhMWY1MmIzOTQyNTRjLi40NzNmMjdhMmJiZGUyZWJiNTg5Y2ZiNzUxYzkyOGI4 Nzc4NDMwMWE3IDEwMDY0NAotLS0gYS9Tb3VyY2UvY21ha2UvV2ViS2l0RmVhdHVyZXMuY21ha2UK KysrIGIvU291cmNlL2NtYWtlL1dlYktpdEZlYXR1cmVzLmNtYWtlCkBAIC0xNzIsNiArMTcyLDcg QEAgbWFjcm8oV0VCS0lUX09QVElPTl9CRUdJTikKICAgICBXRUJLSVRfT1BUSU9OX0RFRklORShF TkFCTEVfUkVTT1VSQ0VfVElNSU5HICJUb2dnbGUgUmVzb3VyY2UgVGltaW5nIHN1cHBvcnQiIFBS SVZBVEUgT0ZGKQogICAgIFdFQktJVF9PUFRJT05fREVGSU5FKEVOQUJMRV9SRVNPVVJDRV9VU0FH RSAiVG9nZ2xlIHJlc291cmNlIHVzYWdlIHN1cHBvcnQiIFBSSVZBVEUgT0ZGKQogICAgIFdFQktJ VF9PUFRJT05fREVGSU5FKEVOQUJMRV9SVUJCRVJfQkFORElORyAiVG9nZ2xlIHJ1YmJlciBiYW5k aW5nIHN1cHBvcnQiIFBSSVZBVEUgT0ZGKQorICAgIFdFQktJVF9PUFRJT05fREVGSU5FKEVOQUJM RV9TQU1QTElOR19QUk9GSUxFUiAiVG9nZ2xlIHNhbXBsaW5nIHByb2ZpZXIgc3VwcG9ydCIgUFJJ VkFURSBPTikKICAgICBXRUJLSVRfT1BUSU9OX0RFRklORShFTkFCTEVfU0VDQ09NUF9GSUxURVJT ICJUb2dnbGUgTGludXggc2VjY29tcCBmaWx0ZXJzIGZvciB0aGUgV2ViUHJvY2VzcyBzdXBwb3J0 IiBQUklWQVRFIE9GRikKICAgICBXRUJLSVRfT1BUSU9OX0RFRklORShFTkFCTEVfU0VSVklDRV9D T05UUk9MUyAiVG9nZ2xlIHNlcnZpY2UgY29udHJvbHMgc3VwcG9ydCIgUFJJVkFURSBPRkYpCiAg ICAgV0VCS0lUX09QVElPTl9ERUZJTkUoRU5BQkxFX1NIQURPV19ET00gIlRvZ2dsZSBzaGFkb3cg ZG9tIiBQUklWQVRFIE9GRikKZGlmZiAtLWdpdCBhL0NoYW5nZUxvZyBiL0NoYW5nZUxvZwppbmRl eCBmOWIzYzI2ZWEwMGUxZjZkM2E2NWViMTk1ZmI4N2NmYTBhYWEyZWI5Li4xYjYzYzUyZjRkMGI2 NzQ1ZGU1MTkzMzczZDAxODhjYWEwOGZmMDMxIDEwMDY0NAotLS0gYS9DaGFuZ2VMb2cKKysrIGIv Q2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTYtMDItMDcgIFl1c3VrZSBTdXp1a2kgIDx1 dGF0YW5lLnRlYUBnbWFpbC5jb20+CisKKyAgICAgICAgW0dUS11bRUZMXSBFbmFibGUgU2FtcGxp bmdQcm9maWxlcgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTUzNjM4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgRW5hYmxlIFNhbXBsaW5nUHJvZmlsZXIgaW4gR1RLIGFuZCBFRkwuCisgICAgICAgIEFuZCBh ZGRlZCBvcHRpb24gdG8gQ01ha2UgdG8gc3dpdGNoIHRoaXMgZnJvbSB0aGUgYnVpbGQgY29tbWFu ZC4KKworICAgICAgICAqIFNvdXJjZS9jbWFrZS9PcHRpb25zRWZsLmNtYWtlOgorICAgICAgICAq IFNvdXJjZS9jbWFrZS9PcHRpb25zR1RLLmNtYWtlOgorICAgICAgICAqIFNvdXJjZS9jbWFrZS9X ZWJLaXRGZWF0dXJlcy5jbWFrZToKKwogMjAxNi0wMi0wNCAgQ3NhYmEgT3N6dHJvZ29uw6FjICA8 b3NzeUB3ZWJraXQub3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQgbm9uIFg4Nl82NCBidWlsZGZp eCBhZnRlciByMTk2MDc3Lgo=