151301 2015-11-15 22:35:52 -0800 Remove unnecessary PLATFORM(EFL) macro in globalMemoryStatistics() 2015-11-19 04:59:02 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 gyuyoung.kim gyuyoung.kim commit-queue ggaren keith_miller mark.lam msaboff ossy saam oldest_to_newest 1142161 0 gyuyoung.kim 2015-11-15 22:35:52 -0800 EXECUTABLE_ALLOCATOR_FIXED is enabled on EFL port. So we don't need to keep PLATFORM(EFL) macro anymore. 1142162 1 265570 gyuyoung.kim 2015-11-15 22:36:43 -0800 Created attachment 265570 Patch 1142206 2 265570 ossy 2015-11-16 03:02:21 -0800 Comment on attachment 265570 Patch I agree to remove (PLATFORM(EFL) && ENABLE(JIT)), but I think we could remove the whole #else case. I don't understand how ExecutableAllocator::committedByteCount()is related to ENABLE(EXECUTABLE_ALLOCATOR_FIXED) guard. ENABLE(EXECUTABLE_ALLOCATOR_FIXED) is enabled only id CPU(X86_64) || PLATFORM(IOS) || CPU(ARM64), so it isn't enabled at least on x86, ARM Linux. But why do we have any guard for using ExecutableAllocator::committedByteCount() ? It is implemented if ENABLE(EXECUTABLE_ALLOCATOR_FIXED) is true or false. 1142432 3 gyuyoung.kim 2015-11-16 18:35:23 -0800 (In reply to comment #2) > Comment on attachment 265570 [details] > Patch > > I agree to remove (PLATFORM(EFL) && ENABLE(JIT)), but I think we could > remove the whole #else case. > > I don't understand how ExecutableAllocator::committedByteCount()is related > to ENABLE(EXECUTABLE_ALLOCATOR_FIXED) guard. > > ENABLE(EXECUTABLE_ALLOCATOR_FIXED) is enabled only id CPU(X86_64) || > PLATFORM(IOS) || CPU(ARM64), > so it isn't enabled at least on x86, ARM Linux. > > But why do we have any guard for using > ExecutableAllocator::committedByteCount() ? > It is implemented if ENABLE(EXECUTABLE_ALLOCATOR_FIXED) is true or false. committedByteCount is supported both by ENABLE_EXECUTABLE_ALLOCATOR_DEMAND and ENABLE_EXECUTABLE_ALLOCATOR_FIXED. So it looks almost ports use one of both at least. I wonder what JS guys think about it. size_t ExecutableAllocator::committedByteCount() { return DemandExecutableAllocator::bytesCommittedByAllocactors(); } 1142744 4 gyuyoung.kim 2015-11-17 18:12:28 -0800 (In reply to comment #2) > Comment on attachment 265570 [details] > Patch > > I agree to remove (PLATFORM(EFL) && ENABLE(JIT)), but I think we could > remove the whole #else case. > > I don't understand how ExecutableAllocator::committedByteCount()is related > to ENABLE(EXECUTABLE_ALLOCATOR_FIXED) guard. > > ENABLE(EXECUTABLE_ALLOCATOR_FIXED) is enabled only id CPU(X86_64) || > PLATFORM(IOS) || CPU(ARM64), > so it isn't enabled at least on x86, ARM Linux. > > But why do we have any guard for using > ExecutableAllocator::committedByteCount() ? > It is implemented if ENABLE(EXECUTABLE_ALLOCATOR_FIXED) is true or false. Ossy, do you think we should land this patch with removing #else statement ? 1142789 5 ossy 2015-11-18 02:30:33 -0800 (In reply to comment #4) > Ossy, do you think we should land this patch with removing #else statement ? I meant the following change: - #if ENABLE(EXECUTABLE_ALLOCATOR_FIXED) || (PLATFORM(EFL) && ENABLE(JIT)) stats.JITBytes = ExecutableAllocator::committedByteCount(); - #else - stats.JITBytes = 0; 1142795 6 265739 gyuyoung.kim 2015-11-18 02:55:24 -0800 Created attachment 265739 Patch 1143007 7 gyuyoung.kim 2015-11-18 15:13:43 -0800 Ossy ping ? 1143249 8 265739 commit-queue 2015-11-19 03:58:43 -0800 Comment on attachment 265739 Patch Clearing flags on attachment: 265739 Committed r192624: <http://trac.webkit.org/changeset/192624> 1143250 9 commit-queue 2015-11-19 03:58:48 -0800 All reviewed patches have been landed. Closing bug. 1143257 10 ossy 2015-11-19 04:20:51 -0800 (In reply to comment #8) > Comment on attachment 265739 [details] > Patch > > Clearing flags on attachment: 265739 > > Committed r192624: <http://trac.webkit.org/changeset/192624> Oh, it broke the CLOOP build, I'll fix it immediately. /Volumes/Data/slave/yosemite-cloop-debug/build/Source/JavaScriptCore/runtime/MemoryStatistics.cpp:40:22: error: use of undeclared identifier 'ExecutableAllocator' stats.JITBytes = ExecutableAllocator::committedByteCount(); ^ 1 error generated. 1143265 11 ossy 2015-11-19 04:59:02 -0800 Platform.h: ------------ ... #if ENABLE(ASSEMBLER) #if CPU(X86_64) || PLATFORM(IOS) || CPU(ARM64) #define ENABLE_EXECUTABLE_ALLOCATOR_FIXED 1 #else #define ENABLE_EXECUTABLE_ALLOCATOR_DEMAND 1 #endif #endif ... So ENABLE(EXECUTABLE_ALLOCATOR_FIXED) || ENABLE(EXECUTABLE_ALLOCATOR_DEMAND) is true only if ENABLE(ASSEMBLER) is true. Fix landed in http://trac.webkit.org/changeset/192626 . 265570 2015-11-15 22:36:43 -0800 2015-11-18 02:55:18 -0800 Patch bug-151301-20151116153634.patch text/plain 1571 gyuyoung.kim U3VidmVyc2lvbiBSZXZpc2lvbjogMTkyNDY0CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA0 Njk0MDA0OGRkYjZiMmUwNjNiZThjOWVkZjFiNDg4NGJmMDFlOTk3Li42MGI5OTA2YjE2YTNmZTQ1 MWRiMmU0NGRhOTg4YzQ3YmY3YzBjYWU4IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNiBAQAorMjAxNS0xMS0xNSAgR3l1eW91bmcgS2ltICA8Z3l1eW91bmcua2ltQHdlYmtp dC5vcmc+CisKKyAgICAgICAgUmVtb3ZlIHVubmVjZXNzYXJ5IFBMQVRGT1JNKEVGTCkgbWFjcm8g aW4gZ2xvYmFsTWVtb3J5U3RhdGlzdGljcygpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xNTEzMDEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBFWEVDVVRBQkxFX0FMTE9DQVRPUl9GSVhFRCBpcyBlbmFibGVk IG9uIEVGTCBwb3J0LiBTbyB3ZSBkb24ndCBuZWVkIHRvIGtlZXAKKyAgICAgICAgUExBVEZPUk0o RUZMKSBtYWNybyBhbnltb3JlLgorCisgICAgICAgICogcnVudGltZS9NZW1vcnlTdGF0aXN0aWNz LmNwcDoKKyAgICAgICAgKEpTQzo6Z2xvYmFsTWVtb3J5U3RhdGlzdGljcyk6CisKIDIwMTUtMTEt MTUgIEFkYW0gQmVyZ2t2aXN0ICA8YWRhbS5iZXJna3Zpc3RAZXJpY3Nzb24uY29tPgogCiAgICAg ICAgIFdlYlJUQzogVXBkYXRlIFJUQ1BlZXJDb25uZWN0aW9uIEFQSSBhbmQgaW50cm9kdWNlIFBl ZXJDb25uZWN0aW9uQmFja2VuZApkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1 bnRpbWUvTWVtb3J5U3RhdGlzdGljcy5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGlt ZS9NZW1vcnlTdGF0aXN0aWNzLmNwcAppbmRleCBhMmVhZjM0OWM1NzFiOThiNjE4ZTY0ODY3NjA2 MzEwNzMyY2I1MzJhLi5hNWEyOWRjMzNlMTM3N2RlZGExY2NjOWZkNWIwYTdjMGFlYTJmYTc1IDEw MDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9NZW1vcnlTdGF0aXN0aWNz LmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9NZW1vcnlTdGF0aXN0aWNz LmNwcApAQCAtMzcsNyArMzcsNyBAQCBHbG9iYWxNZW1vcnlTdGF0aXN0aWNzIGdsb2JhbE1lbW9y eVN0YXRpc3RpY3MoKQogICAgIEdsb2JhbE1lbW9yeVN0YXRpc3RpY3Mgc3RhdHM7CiAKICAgICBz dGF0cy5zdGFja0J5dGVzID0gSlNTdGFjazo6Y29tbWl0dGVkQnl0ZUNvdW50KCk7Ci0jaWYgRU5B QkxFKEVYRUNVVEFCTEVfQUxMT0NBVE9SX0ZJWEVEKSB8fCAoUExBVEZPUk0oRUZMKSAmJiBFTkFC TEUoSklUKSkKKyNpZiBFTkFCTEUoRVhFQ1VUQUJMRV9BTExPQ0FUT1JfRklYRUQpCiAgICAgc3Rh dHMuSklUQnl0ZXMgPSBFeGVjdXRhYmxlQWxsb2NhdG9yOjpjb21taXR0ZWRCeXRlQ291bnQoKTsK ICNlbHNlCiAgICAgc3RhdHMuSklUQnl0ZXMgPSAwOwo= 265739 2015-11-18 02:55:24 -0800 2015-11-19 03:58:43 -0800 Patch bug-151301-20151118195512.patch text/plain 1715 gyuyoung.kim U3VidmVyc2lvbiBSZXZpc2lvbjogMTkyNTY4CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCA4 OThlOTJhMWE1ZWUzMzg0ZWIzZjFlOTM3M2QxMTJlYzIwYmUwNmZlLi5iYTA2M2ZiZTYyZGYwN2Ex MDkyYWUxMzcyZGZlZWViMmIzZDU3NTQ5IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOCBAQAorMjAxNS0xMS0xOCAgR3l1eW91bmcgS2ltICA8Z3l1eW91bmcua2ltQHdlYmtp dC5vcmc+CisKKyAgICAgICAgUmVtb3ZlIHVubmVjZXNzYXJ5IFBMQVRGT1JNKEVGTCkgbWFjcm8g aW4gZ2xvYmFsTWVtb3J5U3RhdGlzdGljcygpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0xNTEzMDEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBFWEVDVVRBQkxFX0FMTE9DQVRPUl9GSVhFRCBpcyBlbmFibGVk IG9uIEVGTCBwb3J0LiBTbyB3ZSBkb24ndCBuZWVkIHRvIGtlZXAKKyAgICAgICAgUExBVEZPUk0o RUZMKSBtYWNybyBhbnltb3JlLiBCZXNpZGVzIEVOQUJMRShFWEVDVVRBQkxFX0FMTE9DQVRPUl9G SVhFRCkgZ3VhcmQKKyAgICAgICAgaXNuJ3QgcmVsYXRlZCB3aXRoIEV4ZWN1dGFibGVBbGxvY2F0 b3I6OmNvbW1pdHRlZEJ5dGVDb3VudCgpIGNsb3NlbHkuIFNvIHRoaXMgcGF0Y2gKKyAgICAgICAg cmVtb3ZlcyBpdCBhcyB3ZWxsLgorCisgICAgICAgICogcnVudGltZS9NZW1vcnlTdGF0aXN0aWNz LmNwcDoKKyAgICAgICAgKEpTQzo6Z2xvYmFsTWVtb3J5U3RhdGlzdGljcyk6CisKIDIwMTUtMTEt MTcgIEJlbmphbWluIFBvdWxhaW4gIDxicG91bGFpbkBhcHBsZS5jb20+CiAKICAgICAgICAgW0pT Q10gVW50YW5nbGUgdGhlIGRlcGVuZGVuY2llcyBvbiBGVExBYmJyZXZpYXRpb25zIGEgYml0CmRp ZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9NZW1vcnlTdGF0aXN0aWNz LmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL01lbW9yeVN0YXRpc3RpY3MuY3Bw CmluZGV4IGEyZWFmMzQ5YzU3MWI5OGI2MThlNjQ4Njc2MDYzMTA3MzJjYjUzMmEuLmVmNzBhM2Rl YTlmYmFjOWIwNzU1NjQwYjI2MjkyZThlMjFjNjhlNGYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9ydW50aW1lL01lbW9yeVN0YXRpc3RpY3MuY3BwCisrKyBiL1NvdXJjZS9KYXZh U2NyaXB0Q29yZS9ydW50aW1lL01lbW9yeVN0YXRpc3RpY3MuY3BwCkBAIC0zNywxMSArMzcsOCBA QCBHbG9iYWxNZW1vcnlTdGF0aXN0aWNzIGdsb2JhbE1lbW9yeVN0YXRpc3RpY3MoKQogICAgIEds b2JhbE1lbW9yeVN0YXRpc3RpY3Mgc3RhdHM7CiAKICAgICBzdGF0cy5zdGFja0J5dGVzID0gSlNT dGFjazo6Y29tbWl0dGVkQnl0ZUNvdW50KCk7Ci0jaWYgRU5BQkxFKEVYRUNVVEFCTEVfQUxMT0NB VE9SX0ZJWEVEKSB8fCAoUExBVEZPUk0oRUZMKSAmJiBFTkFCTEUoSklUKSkKICAgICBzdGF0cy5K SVRCeXRlcyA9IEV4ZWN1dGFibGVBbGxvY2F0b3I6OmNvbW1pdHRlZEJ5dGVDb3VudCgpOwotI2Vs c2UKLSAgICBzdGF0cy5KSVRCeXRlcyA9IDA7Ci0jZW5kaWYKKwogICAgIHJldHVybiBzdGF0czsK IH0KIAo=