175150 2017-08-03 13:00:55 -0700 [Linux] Clear WasmMemory with madvice instead of memset 2017-08-08 06:11:05 -0700 1 1 1 Unclassified WebKit JavaScriptCore Safari Technology Preview Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=175140 https://bugs.webkit.org/show_bug.cgi?id=170343 InRadar P2 Normal --- 1 clopez clopez bugs-noreply buildbot fpizlo ggaren keith_miller mark.lam mcatanzaro msaboff saam webkit-bug-importer ysuzuki zan oldest_to_newest 1334899 0 clopez 2017-08-03 13:00:55 -0700 The test wasm/js-api/test_memory_constructor.js requires lot of RAM to run. On Linux/WebKitGTK+ it currently peaks around 8GB of RSS and spawns 6 subtests at the same time, therefore it requires ~48 GB of RAM to run without trouble. On Mac it peaks around 2.5GB of RSS and also spawns 6 subtests (So I guess it will run fine there with 16GB of RAM). 1334901 1 317140 clopez 2017-08-03 13:04:23 -0700 Created attachment 317140 Patch 1335194 2 317140 clopez 2017-08-04 02:52:55 -0700 Comment on attachment 317140 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317140&action=review > JSTests/wasm/js-api/test_memory_constructor.js:5 > // FIXME: use the assert library: https://bugs.webkit.org/show_bug.cgi?id=165684 > +//@ skip if $memoryLimited > import Builder from '../Builder.js'; > > function assert(b) { I know this kind of patches (actually gardening related) don't need review. But I wanted some feedback/review in this specific case. This test is causing OOM related problems on the Linux bots (We are now running the JSC tests with --memory limited after https://bugs.webkit.org/show_bug.cgi?id=175140 ). So, I want to land this ASAP and I will land this patch now unreviewed. Feedback is still welcome (and of course we are always on time to rework or revert the patch if some of you don't think this change is right) 1335199 3 clopez 2017-08-04 03:01:26 -0700 Committed r220265: <http://trac.webkit.org/changeset/220265> 1335201 4 webkit-bug-importer 2017-08-04 03:02:16 -0700 <rdar://problem/33720817> 1335365 5 317140 saam 2017-08-04 11:42:35 -0700 Comment on attachment 317140 Patch Carlos, is the fast memory optimization enabled on Linux? If so, we’ll allocate about 4GB for a WebAssembly memory if we can. A good place to start is o see if we enable this on Linux. If so, that’s eating up a lot of virtual memory. 1335366 6 ysuzuki 2017-08-04 11:46:18 -0700 (In reply to Saam Barati from comment #5) > Comment on attachment 317140 [details] > Patch > > Carlos, is the fast memory optimization enabled on Linux? If so, we’ll > allocate about 4GB for a WebAssembly memory if we can. A good place to start > is o see if we enable this on Linux. If so, that’s eating up a lot of > virtual memory. Yes, it is enabled in Linux ports. 1335426 7 saam 2017-08-04 13:40:14 -0700 (In reply to Yusuke Suzuki from comment #6) > (In reply to Saam Barati from comment #5) > > Comment on attachment 317140 [details] > > Patch > > > > Carlos, is the fast memory optimization enabled on Linux? If so, we’ll > > allocate about 4GB for a WebAssembly memory if we can. A good place to start > > is o see if we enable this on Linux. If so, that’s eating up a lot of > > virtual memory. > > Yes, it is enabled in Linux ports. It seems likely this is why a lot of memory is being used. That said, it should be just virtual memory, and hopefully not need actual 48GB physical memory. Has this recently regressed? Or has it always been an issue? Maybe it's gigacage related since that code touched the Wasm memory allocation code path. Maybe we're missing a syscall letting the kernel know that it doesn't actually need to allocate physical memory for these pages. 1335439 8 ysuzuki 2017-08-04 13:59:08 -0700 (In reply to Saam Barati from comment #7) > (In reply to Yusuke Suzuki from comment #6) > > (In reply to Saam Barati from comment #5) > > > Comment on attachment 317140 [details] > > > Patch > > > > > > Carlos, is the fast memory optimization enabled on Linux? If so, we’ll > > > allocate about 4GB for a WebAssembly memory if we can. A good place to start > > > is o see if we enable this on Linux. If so, that’s eating up a lot of > > > virtual memory. > > > > Yes, it is enabled in Linux ports. > > It seems likely this is why a lot of memory is being used. That said, it > should be just virtual memory, and hopefully not need actual 48GB physical > memory. Has this recently regressed? Or has it always been an issue? Maybe > it's gigacage related since that code touched the Wasm memory allocation > code path. Maybe we're missing a syscall letting the kernel know that it > doesn't actually need to allocate physical memory for these pages. I've just checked and I think it is regression. Actually, RSS becomes 4GB~ in Linux jsc process. I'm looking into this. 1335516 9 ysuzuki 2017-08-04 15:12:21 -0700 (In reply to Yusuke Suzuki from comment #8) > I've just checked and I think it is regression. > Actually, RSS becomes 4GB~ in Linux jsc process. I'm looking into this. In WasmMemory, in fast memory case, we use memset to initialize memory. Yeah, but of course, it populates backing store. Is it intentional? I wonder if this also populates memory in Darwin. https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/wasm/WasmMemory.cpp#L305 1335535 10 ysuzuki 2017-08-04 15:29:38 -0700 (In reply to Yusuke Suzuki from comment #9) > (In reply to Yusuke Suzuki from comment #8) > > I've just checked and I think it is regression. > > Actually, RSS becomes 4GB~ in Linux jsc process. I'm looking into this. > > In WasmMemory, in fast memory case, we use memset to initialize memory. > Yeah, but of course, it populates backing store. Is it intentional? > I wonder if this also populates memory in Darwin. > > https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/wasm/ > WasmMemory.cpp#L305 I think 1. When freeing large memory with AllocationKind == virtual, we should set LargeRange with physicalSize = size because virtual memory could be populated anyway. 2. Do not call memset in WasmMemory. We should rely on bmalloc's large memory allocation. It is responsibility of bmalloc to zero-fill this virtual memory area without populating backing pages. In Linux case, we should call madvice(MADV_DONTNEED). Fil, Saam and Geoff, what do you think of? 1335707 11 ysuzuki 2017-08-05 10:52:47 -0700 Reopening to attach new patch. 1335708 12 317342 ysuzuki 2017-08-05 10:52:49 -0700 Created attachment 317342 Patch 1335812 13 317375 ysuzuki 2017-08-06 12:09:59 -0700 Created attachment 317375 Patch 1335826 14 317375 clopez 2017-08-06 12:56:46 -0700 Comment on attachment 317375 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317375&action=review > JSTests/wasm/js-api/test_memory_constructor.js:4 > // FIXME: use the assert library: https://bugs.webkit.org/show_bug.cgi?id=165684 > -//@ skip if $memoryLimited > import Builder from '../Builder.js'; > > function assert(b) { When this test runs it spawns 6 jsc process (6 subtests). The difference between this 6 subtests is on the parameter passed: By checking the ps output I see the base arguments for all 6 subtests are: "--useFTLJIT=true --useFunctionDotArguments=true --maxPerThreadStackUsage=1572864 -m" And then, the difference on the arguments (from the base ones) for each one of this 6 subtests are: Subtest 1: Pass no extra paremeter. Subtest 2: Pass --forceCodeBlockToJettisonDueToOldAge=true Subtest 3: Pass --useCallICsForWebAssemblyToJSCalls=false Subtest 4: Pass --useFastTLSForWasmContext=true --useConcurrentJIT=false --thresholdForJITAfterWarmUp=100 --scribbleFreeCells=true Subtest 5: Pass --useFastTLSForWasmContext=false Subtest 6: Pass --useWebAssemblyFastMemory=false Before this patch, all 6 subtests where allocating 8GB of RSS. I have tested the patch and after it, 5 of the 6 subtests now allocate only a few MB of RSS (instead of 8GB). So this is amazing! :) But there is still one that keeps using 8GB of RAM of RSS (Resident Set Size). And its the 6# one, the one that runs with --useWebAssemblyFastMemory=false You can check it manually by doing this: # Terminal 1 $ while true; do if ps x -o pid,rss,vsz,command | grep js[c]|grep -v python|grep -v ruby; then free -m; echo ; fi;done # Terminal 2 $ cd WebKit $ cd JSTests/wasm $ ../../WebKitBuild/Release/bin/jsc --useFTLJIT=true --useFunctionDotArguments=true --maxPerThreadStackUsage=1572864 -m --useWebAssemblyFastMemory=false js-api/test_memory_constructor.js You should see on the other terminal how the jsc process still peaks at 8GB of RSS for the JSC process. If we can fix this also this 6th case, then we can remove the "@ skip if $memoryLimited" line. But otherwise maybe is better to keep it, as running this test still requires 8GB of RAM (which I guess is more than what a memorylimited machine can use without issues) 1335828 15 clopez 2017-08-06 13:16:52 -0700 (In reply to Yusuke Suzuki from comment #9) > (In reply to Yusuke Suzuki from comment #8) > > I've just checked and I think it is regression. > > Actually, RSS becomes 4GB~ in Linux jsc process. I'm looking into this. > > In WasmMemory, in fast memory case, we use memset to initialize memory. > Yeah, but of course, it populates backing store. Is it intentional? > I wonder if this also populates memory in Darwin. > > https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/wasm/ > WasmMemory.cpp#L305 I think this also populates memory on Darwin. Right? I will be really surprised if this is not the case. When I tested this on a mac, I saw each one of the 6 subtests peaking at 2.5GB of RSS (on Mac). On Linux they peak at 8GB of RSS. After the patch https://bugs.webkit.org/attachment.cgi?id=317375 5 of the 6 subtests (the ones that run with useWebAssemblyFastMemory=true) only use a few MBs of RSS (less than 20MB). But on Mac this 5 tests still allocate 2.5GB of RSS (I believe, I don't have a Mac at hand now, I may confirm this tomorrow) 1335891 16 317404 ysuzuki 2017-08-06 23:46:57 -0700 Created attachment 317404 Patch 1335892 17 ysuzuki 2017-08-06 23:49:00 -0700 I keep memoryLimited annotation since this patch only solves the problem under fast memory. I don't think using madvise to non fast memory thing is not good solution. So, I keep the current one as is. But it is not a problem since Linux ports always use fast memory right now. And fast memory will not be disabled if WebKitGTK+ does not support WebKitLegacy. 1335925 18 clopez 2017-08-07 05:08:14 -0700 (In reply to Carlos Alberto Lopez Perez from comment #15) > (In reply to Yusuke Suzuki from comment #9) > > (In reply to Yusuke Suzuki from comment #8) > > > I've just checked and I think it is regression. > > > Actually, RSS becomes 4GB~ in Linux jsc process. I'm looking into this. > > > > In WasmMemory, in fast memory case, we use memset to initialize memory. > > Yeah, but of course, it populates backing store. Is it intentional? > > I wonder if this also populates memory in Darwin. > > > > https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/wasm/ > > WasmMemory.cpp#L305 > > I think this also populates memory on Darwin. Right? I will be really > surprised if this is not the case. > > When I tested this on a mac, I saw each one of the 6 subtests peaking at > 2.5GB of RSS (on Mac). On Linux they peak at 8GB of RSS. > > > After the patch https://bugs.webkit.org/attachment.cgi?id=317375 5 of the 6 > subtests (the ones that run with useWebAssemblyFastMemory=true) only use a > few MBs of RSS (less than 20MB). But on Mac this 5 tests still allocate > 2.5GB of RSS (I believe, I don't have a Mac at hand now, I may confirm this > tomorrow) Confirmed. Running this test on Mac also requires lot of RAM. So I guess this means that memset(0) also causes allocation of real RAM on Darwin (seems logic). Each one of this 6 subtests want to use 8GB of RSS also on Mac, but since the Mac that I have for tests "only" has 16GB of RAM and there are 6 subtests on parallel they only get to use 2.5GB (16/6) of RSS each one before the Darwin kernel does something with them (Killing them?). On the dmesg log this appears for all of them: CODE SIGNING: cs_invalid_page(0x10daeb000): p=11709[jsc] final status 0x0, allowing (remove VALID) page No idea what that means. So: the same issue that happens on Linux seems to happen on Mac. But on Mac an OOM situation doesn't cause a bad user experience (desktop freezes) like on Linux. I tested this by running: 1) Terminal 1 $ while true; d if ps x -o pid,rss,vsz,command | grep js[c]|grep -v python|grep -v ruby; then top -l 1 -s 0 | grep PhysMem; echo ; fi;done 2) Terminal 2 $ Tools/Scripts/run-javascriptcore-tests --release --no-build --no-testapi --no-testmasm --no-testair --no-testb3 --no-mozilla-tests --no-quick --filter js-api/test_memory_constructor.js The attached patch here makes the situation much better on Linux, as all the 5 subtests that run with useWebAssemblyFastMemory=true now only need to allocate a few MBs (less than 20MB). On mac the patch makes no difference, is neutral there. 1335926 19 clopez 2017-08-07 05:09:35 -0700 (In reply to Carlos Alberto Lopez Perez from comment #18) > I tested this by running: > > 1) Terminal 1 > $ while true; d if ps x -o pid,rss,vsz,command | grep js[c]|grep -v > python|grep -v ruby; then top -l 1 -s 0 | grep PhysMem; echo ; fi;done > while true; do if ps x -o pid,rss,vsz,command | grep js[c]|grep -v python|grep -v ruby; then top -l 1 -s 0 | grep PhysMem; echo ; fi; done 1335927 20 ysuzuki 2017-08-07 05:22:22 -0700 This patch intends to fix OOM issue in Linux. So, the change is guarded by OS(LINUX). In the different OSes, no change is made by this patch. 1335928 21 clopez 2017-08-07 05:39:35 -0700 (In reply to Yusuke Suzuki from comment #20) > This patch intends to fix OOM issue in Linux. So, the change is guarded by > OS(LINUX). In the different OSes, no change is made by this patch. Right. The patch is a really good improvement and I'll r+ it tomorrow if no further comments by Saam or someone else. But I was wondering why using memset(0) here for Mac? Is just because on Mac you can trigger an OOM condition without having to fear to bring down or freeze the system? That don't seems right to me. 1335929 22 clopez 2017-08-07 05:41:37 -0700 (In reply to Carlos Alberto Lopez Perez from comment #21) > But I was wondering why using memset(0) here for Mac? Is just because on Mac > you can trigger an OOM condition without having to fear to bring down or > freeze the system? That don't seems right to me. To be clear: this question is not related with this patch, but more general wondering about why the current code is doing this. Using memset(0) is what is currently happening before any patch 1335934 23 clopez 2017-08-07 06:04:09 -0700 (In reply to Carlos Alberto Lopez Perez from comment #22) > (In reply to Carlos Alberto Lopez Perez from comment #21) > > But I was wondering why using memset(0) here for Mac? Is just because on Mac > > you can trigger an OOM condition without having to fear to bring down or > > freeze the system? That don't seems right to me. > > To be clear: this question is not related with this patch, but more general > wondering about why the current code is doing this. > > Using memset(0) is what is currently happening before any patch A few git blames reveal this: https://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wasm/WasmMemory.cpp?rev=219595#L69 which was then removed (the comment) on r220118 <https://trac.webkit.org/r220118> So bug 170343 seems related. 1335940 24 mcatanzaro 2017-08-07 07:02:16 -0700 (In reply to Yusuke Suzuki from comment #17) > I keep memoryLimited annotation since this patch only solves the problem > under fast memory. > I don't think using madvise to non fast memory thing is not good solution. > So, I keep the current one as is. But it is not a problem since Linux ports > always use fast memory right now. And fast memory will not be disabled if > WebKitGTK+ does not support WebKitLegacy. Not sure if I understand this correctly, but: we do support using system malloc, and we have distributors that depend on it because bmalloc does not work on certain architectures. 1335944 25 ysuzuki 2017-08-07 07:10:05 -0700 (In reply to Michael Catanzaro from comment #24) > (In reply to Yusuke Suzuki from comment #17) > > I keep memoryLimited annotation since this patch only solves the problem > > under fast memory. > > I don't think using madvise to non fast memory thing is not good solution. > > So, I keep the current one as is. But it is not a problem since Linux ports > > always use fast memory right now. And fast memory will not be disabled if > > WebKitGTK+ does not support WebKitLegacy. > > Not sure if I understand this correctly, but: we do support using system > malloc, and we have distributors that depend on it because bmalloc does not > work on certain architectures. If we use system malloc, our Gigacage::tryAllocateVirtualPages just execute OSAllocator. And our commitZeroPages in this patch just works fine. So, both is ok. 1335947 26 ysuzuki 2017-08-07 07:42:46 -0700 (In reply to Carlos Alberto Lopez Perez from comment #22) > (In reply to Carlos Alberto Lopez Perez from comment #21) > > But I was wondering why using memset(0) here for Mac? Is just because on Mac > > you can trigger an OOM condition without having to fear to bring down or > > freeze the system? That don't seems right to me. > > To be clear: this question is not related with this patch, but more general > wondering about why the current code is doing this. > > Using memset(0) is what is currently happening before any patch As is described in bug 170343, changing the strategy based on required memory size would be promising. But, as a first step, doing on-demand-paging in this patch and then deciding the threshold sounds better. 1335951 27 317404 saam 2017-08-07 08:14:07 -0700 Comment on attachment 317404 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317404&action=review > Source/JavaScriptCore/wasm/WasmMemory.cpp:263 > + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. 1335957 28 317404 ysuzuki 2017-08-07 08:44:18 -0700 Comment on attachment 317404 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317404&action=review >> Source/JavaScriptCore/wasm/WasmMemory.cpp:263 >> + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } > > This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. That's why we immediately execute OSAllocator::commit after that. 1335959 29 317404 ysuzuki 2017-08-07 08:50:50 -0700 Comment on attachment 317404 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317404&action=review >>> Source/JavaScriptCore/wasm/WasmMemory.cpp:263 >>> + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } >> >> This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. > > That's why we immediately execute OSAllocator::commit after that. For the case of the small size, memset(...) would be nice here. I don't think memset all the area of this here, it exhausts memory even if it is not used. The threshold should be decided based on the result of some benchmark. 1335960 30 317404 ysuzuki 2017-08-07 08:52:37 -0700 Comment on attachment 317404 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317404&action=review >>>> Source/JavaScriptCore/wasm/WasmMemory.cpp:263 >>>> + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } >>> >>> This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. >> >> That's why we immediately execute OSAllocator::commit after that. > > For the case of the small size, memset(...) would be nice here. > I don't think memset all the area of this here, it exhausts memory even if it is not used. > The threshold should be decided based on the result of some benchmark. I mean, I don't think clearing all the area here by using memset is good way. I t exhausts memory even if it is not used. (And currently, it exhausts memory in Linux). 1335987 31 clopez 2017-08-07 09:34:20 -0700 (In reply to Yusuke Suzuki from comment #30) > Comment on attachment 317404 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=317404&action=review > > >>>> Source/JavaScriptCore/wasm/WasmMemory.cpp:263 > >>>> + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } > >>> > >>> This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. > >> > >> That's why we immediately execute OSAllocator::commit after that. > > > > For the case of the small size, memset(...) would be nice here. > > I don't think memset all the area of this here, it exhausts memory even if it is not used. > > The threshold should be decided based on the result of some benchmark. > > I mean, I don't think clearing all the area here by using memset is good > way. I t exhausts memory even if it is not used. (And currently, it exhausts > memory in Linux). It exhausts memory also on Mac. Basically this test is asking for _two_ memory segments with an _initial_ size of 4GBs. See https://trac.webkit.org/browser/trunk/JSTests/wasm/js-api/test_memory_constructor.js?rev=220265#L44 and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Memory And 6 subtests are executed on parallel. So...., the only way this test can run fine is if your machine has 48GB of RAM. On Mac it throws an error (therefore the messages I saw here https://bugs.webkit.org/show_bug.cgi?id=175150#c18 about dmesg). On Linux it either does the same (throw) or gets killed by the Linux OOM killer. On Mac causing an OOM condition seems more or less fine (you maybe even don't notice). On Linux is not fine. Your computer will freeze for several seconds. A similar issue was discussed on bug 175100 Do we really need to try to allocate so much memory for this test? The Mac JSC bots "only" have 16GBs of RAM. I think this test is really throwing exceptions there because there is not enough memory to run the 6 subtests at the same time. And instead of testing what it was meant to test, its testing that it can throw an error (that is catched) for not enough memory. However, I think this patch is a really interesting approach and can make things better when some application requests more initial memory than it will really needs. Ideally this should be done with some heuristics based on the requested size. If the size is small using memset(0) is probably better than calling MADV_DONTNEED. I'm speculating here as I have no real idea, some benchmark or empiric testing will be needed. 1336010 32 317404 fpizlo 2017-08-07 10:18:33 -0700 Comment on attachment 317404 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317404&action=review This patch seems sensible to me. r=me. >>>>>> Source/JavaScriptCore/wasm/WasmMemory.cpp:263 >>>>>> + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } >>>>> >>>>> This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. >>>> >>>> That's why we immediately execute OSAllocator::commit after that. >>> >>> For the case of the small size, memset(...) would be nice here. >>> I don't think memset all the area of this here, it exhausts memory even if it is not used. >>> The threshold should be decided based on the result of some benchmark. >> >> I mean, I don't think clearing all the area here by using memset is good way. I t exhausts memory even if it is not used. (And currently, it exhausts memory in Linux). > > It exhausts memory also on Mac. > > Basically this test is asking for _two_ memory segments with an _initial_ size of 4GBs. > > See https://trac.webkit.org/browser/trunk/JSTests/wasm/js-api/test_memory_constructor.js?rev=220265#L44 > and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Memory > > And 6 subtests are executed on parallel. So...., the only way this test can run fine is if your machine has 48GB of RAM. > > On Mac it throws an error (therefore the messages I saw here https://bugs.webkit.org/show_bug.cgi?id=175150#c18 about dmesg). > On Linux it either does the same (throw) or gets killed by the Linux OOM killer. > > On Mac causing an OOM condition seems more or less fine (you maybe even don't notice). On Linux is not fine. Your computer will freeze for several seconds. A similar issue was discussed on bug 175100 > > > Do we really need to try to allocate so much memory for this test? The Mac JSC bots "only" have 16GBs of RAM. I think this test is really throwing exceptions there because there is not enough memory to run the 6 subtests at the same time. And instead of testing what it was meant to test, its testing that it can throw an error (that is catched) for not enough memory. > > However, I think this patch is a really interesting approach and can make things better when some application requests more initial memory than it will really needs. Ideally this should be done with some heuristics based on the requested size. If the size is small using memset(0) is probably better than calling MADV_DONTNEED. I'm speculating here as I have no real idea, some benchmark or empiric testing will be needed. I agree with Carlos that we should not have a test that allocates 4GB of RAM, unless some precautions are taken, like putting "//@ runDefault" at the top to make it only run one of the configurations instead of N. Also, instead of allocating 4GB, we should think hard about what we can do to test the same code by requesting less memory. If it's an OOM test, we could add options to force the memory allocator to assume that there is less memory than there really is, so that we can drive the VM to OOM in a more reasonable amount of memory. 1336353 33 saam 2017-08-07 21:42:56 -0700 (In reply to Filip Pizlo from comment #32) > Comment on attachment 317404 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=317404&action=review > > This patch seems sensible to me. r=me. > > >>>>>> Source/JavaScriptCore/wasm/WasmMemory.cpp:263 > >>>>>> + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } > >>>>> > >>>>> This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. > >>>> > >>>> That's why we immediately execute OSAllocator::commit after that. > >>> > >>> For the case of the small size, memset(...) would be nice here. > >>> I don't think memset all the area of this here, it exhausts memory even if it is not used. > >>> The threshold should be decided based on the result of some benchmark. > >> > >> I mean, I don't think clearing all the area here by using memset is good way. I t exhausts memory even if it is not used. (And currently, it exhausts memory in Linux). > > > > It exhausts memory also on Mac. > > > > Basically this test is asking for _two_ memory segments with an _initial_ size of 4GBs. > > > > See https://trac.webkit.org/browser/trunk/JSTests/wasm/js-api/test_memory_constructor.js?rev=220265#L44 > > and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Memory > > > > And 6 subtests are executed on parallel. So...., the only way this test can run fine is if your machine has 48GB of RAM. > > > > On Mac it throws an error (therefore the messages I saw here https://bugs.webkit.org/show_bug.cgi?id=175150#c18 about dmesg). > > On Linux it either does the same (throw) or gets killed by the Linux OOM killer. > > > > On Mac causing an OOM condition seems more or less fine (you maybe even don't notice). On Linux is not fine. Your computer will freeze for several seconds. A similar issue was discussed on bug 175100 > > > > > > Do we really need to try to allocate so much memory for this test? The Mac JSC bots "only" have 16GBs of RAM. I think this test is really throwing exceptions there because there is not enough memory to run the 6 subtests at the same time. And instead of testing what it was meant to test, its testing that it can throw an error (that is catched) for not enough memory. > > > > However, I think this patch is a really interesting approach and can make things better when some application requests more initial memory than it will really needs. Ideally this should be done with some heuristics based on the requested size. If the size is small using memset(0) is probably better than calling MADV_DONTNEED. I'm speculating here as I have no real idea, some benchmark or empiric testing will be needed. > > I agree with Carlos that we should not have a test that allocates 4GB of > RAM, unless some precautions are taken, like putting "//@ runDefault" at the > top to make it only run one of the configurations instead of N. Also, > instead of allocating 4GB, we should think hard about what we can do to test > the same code by requesting less memory. If it's an OOM test, we could add > options to force the memory allocator to assume that there is less memory > than there really is, so that we can drive the VM to OOM in a more > reasonable amount of memory. I don't think the test is stressing OOM as far as I remember. I'm ok with just setting it to @runDefault 1336356 34 saam 2017-08-07 21:44:31 -0700 (In reply to Yusuke Suzuki from comment #28) > Comment on attachment 317404 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=317404&action=review > > >> Source/JavaScriptCore/wasm/WasmMemory.cpp:263 > >> + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } > > > > This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. > > That's why we immediately execute OSAllocator::commit after that. This makes sense. 1336357 35 saam 2017-08-07 21:45:09 -0700 I just reread the test again. It's asking for the largest possible amount of memory that is legal for a WebAssembly.memory. It seems reasonable to only run this once, or to revise the test to be less crazy. 1336358 36 317404 saam 2017-08-07 21:49:58 -0700 Comment on attachment 317404 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317404&action=review > Source/JavaScriptCore/wasm/WasmMemory.cpp:269 > + bool writable = true; > + bool executable = false; Nit: these variables can be defined outside of the #if 1336446 37 317404 ysuzuki 2017-08-08 05:59:53 -0700 Comment on attachment 317404 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317404&action=review >>>>>>>>> Source/JavaScriptCore/wasm/WasmMemory.cpp:263 >>>>>>>>> + while (madvise(startAddress, sizeInBytes, MADV_DONTNEED) == -1 && errno == EAGAIN) { } >>>>>>>> >>>>>>>> This feels wrong. The programmer is telling us they need this memory (most likely because they will run a wasm program and access it), but you’re telling the kernel you don’t expect this memory to be accessed anytime soon. This feels wrong, and it seems possible that it will slow down actual Wasm programs. >>>>>>> >>>>>>> That's why we immediately execute OSAllocator::commit after that. >>>>>> >>>>>> For the case of the small size, memset(...) would be nice here. >>>>>> I don't think memset all the area of this here, it exhausts memory even if it is not used. >>>>>> The threshold should be decided based on the result of some benchmark. >>>>> >>>>> I mean, I don't think clearing all the area here by using memset is good way. I t exhausts memory even if it is not used. (And currently, it exhausts memory in Linux). >>>> >>>> It exhausts memory also on Mac. >>>> >>>> Basically this test is asking for _two_ memory segments with an _initial_ size of 4GBs. >>>> >>>> See https://trac.webkit.org/browser/trunk/JSTests/wasm/js-api/test_memory_constructor.js?rev=220265#L44 >>>> and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WebAssembly/Memory >>>> >>>> And 6 subtests are executed on parallel. So...., the only way this test can run fine is if your machine has 48GB of RAM. >>>> >>>> On Mac it throws an error (therefore the messages I saw here https://bugs.webkit.org/show_bug.cgi?id=175150#c18 about dmesg). >>>> On Linux it either does the same (throw) or gets killed by the Linux OOM killer. >>>> >>>> On Mac causing an OOM condition seems more or less fine (you maybe even don't notice). On Linux is not fine. Your computer will freeze for several seconds. A similar issue was discussed on bug 175100 >>>> >>>> >>>> Do we really need to try to allocate so much memory for this test? The Mac JSC bots "only" have 16GBs of RAM. I think this test is really throwing exceptions there because there is not enough memory to run the 6 subtests at the same time. And instead of testing what it was meant to test, its testing that it can throw an error (that is catched) for not enough memory. >>>> >>>> However, I think this patch is a really interesting approach and can make things better when some application requests more initial memory than it will really needs. Ideally this should be done with some heuristics based on the requested size. If the size is small using memset(0) is probably better than calling MADV_DONTNEED. I'm speculating here as I have no real idea, some benchmark or empiric testing will be needed. >>> >>> I agree with Carlos that we should not have a test that allocates 4GB of RAM, unless some precautions are taken, like putting "//@ runDefault" at the top to make it only run one of the configurations instead of N. Also, instead of allocating 4GB, we should think hard about what we can do to test the same code by requesting less memory. If it's an OOM test, we could add options to force the memory allocator to assume that there is less memory than there really is, so that we can drive the VM to OOM in a more reasonable amount of memory. >> >> I don't think the test is stressing OOM as far as I remember. I'm ok with just setting it to @runDefault > > This makes sense. OK, I filed the separate bug for runWebAssemblyDefault thing. >> Source/JavaScriptCore/wasm/WasmMemory.cpp:269 >> + bool executable = false; > > Nit: these variables can be defined outside of the #if Nice, fixed. 1336453 38 ysuzuki 2017-08-08 06:11:05 -0700 Committed r220401: <http://trac.webkit.org/changeset/220401> 317140 2017-08-03 13:04:23 -0700 2017-08-05 10:52:44 -0700 Patch bug-175150-20170803220422.patch text/plain 1175 clopez U3VidmVyc2lvbiBSZXZpc2lvbjogMjIwMjIxCmRpZmYgLS1naXQgYS9KU1Rlc3RzL0NoYW5nZUxv ZyBiL0pTVGVzdHMvQ2hhbmdlTG9nCmluZGV4IGI2NTk2ZDhiZGRmMGMyMjhiYmJlMTFkNjRjNGY0 ZTIyOGY1NGNlMGUuLjk5NGEyOTMyYTVlZTI1OWM1NjJiNDMxNWE2ZTYxNjAwMmUxZjE0OTEgMTAw NjQ0Ci0tLSBhL0pTVGVzdHMvQ2hhbmdlTG9nCisrKyBiL0pTVGVzdHMvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTIgQEAKKzIwMTctMDgtMDMgIENhcmxvcyBBbGJlcnRvIExvcGV6IFBlcmV6ICA8Y2xv cGV6QGlnYWxpYS5jb20+CisKKyAgICAgICAgSlNDIHRlc3Qgd2FzbS9qcy1hcGkvdGVzdF9tZW1v cnlfY29uc3RydWN0b3IuanMgc2hvdWxkIGJlIHNraXBwZWQgb24gbWVtb3J5TGltaXRlZAorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc1MTUwCisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiB3YXNtL2pzLWFw aS90ZXN0X21lbW9yeV9jb25zdHJ1Y3Rvci5qczoKKwogMjAxNy0wOC0wMiAgQ2FybG9zIEFsYmVy dG8gTG9wZXogUGVyZXogIDxjbG9wZXpAaWdhbGlhLmNvbT4KIAogICAgICAgICBbTGludXhdIEpT VGVzdHMvd2FzbS9zdHJlc3Mvb29tLmpzIHNob3VsZCBub3QgcnVuIG9uIExpbnV4CmRpZmYgLS1n aXQgYS9KU1Rlc3RzL3dhc20vanMtYXBpL3Rlc3RfbWVtb3J5X2NvbnN0cnVjdG9yLmpzIGIvSlNU ZXN0cy93YXNtL2pzLWFwaS90ZXN0X21lbW9yeV9jb25zdHJ1Y3Rvci5qcwppbmRleCBlNjdkZjky YWRmYjk2Y2JlYmJlYzFjMTRjM2RhZmYxZTJkZGU2MTBlLi5iODg5M2VhYjdhZjJmYWMwZWEzNDhj NWVlNTI1NjdjM2NlNTRjNmQ5IDEwMDY0NAotLS0gYS9KU1Rlc3RzL3dhc20vanMtYXBpL3Rlc3Rf bWVtb3J5X2NvbnN0cnVjdG9yLmpzCisrKyBiL0pTVGVzdHMvd2FzbS9qcy1hcGkvdGVzdF9tZW1v cnlfY29uc3RydWN0b3IuanMKQEAgLTEsNCArMSw1IEBACiAvLyBGSVhNRTogdXNlIHRoZSBhc3Nl cnQgbGlicmFyeTogaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2NTY4 NAorLy9AIHNraXAgaWYgJG1lbW9yeUxpbWl0ZWQKIGltcG9ydCBCdWlsZGVyIGZyb20gJy4uL0J1 aWxkZXIuanMnOwogCiBmdW5jdGlvbiBhc3NlcnQoYikgewo= 317342 2017-08-05 10:52:49 -0700 2017-08-06 12:09:57 -0700 Patch bug-175150-20170806025248.patch text/plain 3434 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjIwMzE4CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCBk MGNkZDM5YmQ0YzQ1NGNlNjEwMDk1ZmQzOTlkMjE4Nzc4OWVmNzg1Li45OGQzM2I5NGYyMmE4YjVj NjAwZTVlOGE4MzIwZGQ3Y2VjNGIxYzA5IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMiBAQAorMjAxNy0wOC0wNSAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KKworICAgICAgICBbTGludXhdIENsZWFyIFdhc21NZW1vcnkgd2l0aCBtYWR2aWNlIGlu c3RlYWQgb2YgbWVtc2V0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0xNzUxNTAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBJbiBMaW51eCwgemVyb2luZyBwYWdlcyB3aXRoIG1lbXNldCBwb3B1bGF0ZXMgYmFj a2luZyBzdG9yZS4KKyAgICAgICAgSW5zdGVhZCwgd2Ugc2hvdWxkIHVzZSBtYWR2aXNlIHdpdGgg TUFEVl9ET05UTkVFRC4gSXQgZGlzY2FyZHMKKyAgICAgICAgcGFnZXMuIEFuZCBpZiB5b3UgYWNj ZXNzIHRoZXNlIHBhZ2VzLCBvbi1kZW1hbmQtemVyby1wYWdlcyB3aWxsCisgICAgICAgIGJlIHNo b3duLgorCisgICAgICAgIFdlIGFsc28gY29tbWl0IGdyb3duIHBhZ2VzIGluIGFsbCBPU2VzLgor CisgICAgICAgICogd2FzbS9XYXNtTWVtb3J5LmNwcDoKKyAgICAgICAgKEpTQzo6V2FzbTo6Y29t bWl0WmVyb1BhZ2VzKToKKyAgICAgICAgKEpTQzo6V2FzbTo6TWVtb3J5OjpjcmVhdGUpOgorICAg ICAgICAoSlNDOjpXYXNtOjpNZW1vcnk6Omdyb3cpOgorCiAyMDE3LTA4LTA1ICBNYXJrIExhbSAg PG1hcmsubGFtQGFwcGxlLmNvbT4KIAogICAgICAgICBNb3ZlIERGRzo6T1NSRXhpdENvbXBpbGVy IG1ldGhvZHMgaW50byBERkc6Ok9TUkV4aXQgW3N0ZXAgM10uCmRpZmYgLS1naXQgYS9Tb3VyY2Uv SmF2YVNjcmlwdENvcmUvd2FzbS9XYXNtTWVtb3J5LmNwcCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS93YXNtL1dhc21NZW1vcnkuY3BwCmluZGV4IGRiNzczMWY1N2ZlYTg1NzNhMjYwMWNmY2Y3YTMy ZDI1MmViZTVkZjQuLmFlMjkzMTY2MjE0MDAyMjVmZWM0NWUwZDkzZDczNjMwNjQyZWRjOWQgMTAw NjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93YXNtL1dhc21NZW1vcnkuY3BwCisrKyBi L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93YXNtL1dhc21NZW1vcnkuY3BwCkBAIC0yNTUsNiArMjU1 LDIzIEBAIE1lbW9yeTo6TWVtb3J5KHZvaWQqIG1lbW9yeSwgUGFnZUNvdW50IGluaXRpYWwsIFBh Z2VDb3VudCBtYXhpbXVtLCBzaXplX3QgbWFwcGVkCiAgICAgZGF0YUxvZ0xuSWYodmVyYm9zZSwg Ik1lbW9yeTo6TWVtb3J5IGFsbG9jYXRpbmcgIiwgKnRoaXMpOwogfQogCitzdGF0aWMgdm9pZCBj b21taXRaZXJvUGFnZXModm9pZCogc3RhcnRBZGRyZXNzLCBzaXplX3Qgc2l6ZUluQnl0ZXMpCit7 CisjaWYgT1MoTElOVVgpCisgICAgLy8gSW4gTGludXgsIE1BRFZfRE9OVE5FRUQgY2xlYXJzIGJh Y2tpbmcgcGFnZXMgd2l0aCB6ZXJvLiBCZSBDYXJlZnVsIHRoYXQgTUFEVl9ET05UTkVFRCBzaG93 cyBkaWZmZXJlbnQgc2VtYW50aWNzIGluIGRpZmZlcmVudCBPU2VzLgorICAgIC8vIEZvciBleGFt cGxlLCBGcmVlQlNEIGRvZXMgbm90IGNsZWFyIGJhY2tpbmcgcGFnZXMgaW1tZWRpYXRlbHkuCisg ICAgd2hpbGUgKG1hZHZpc2Uoc3RhcnRBZGRyZXNzLCBzaXplSW5CeXRlcywgTUFEVl9ET05UTkVF RCkgPT0gLTEgJiYgZXJybm8gPT0gRUFHQUlOKSB7IH0KKyAgICBib29sIHdyaXRhYmxlID0gdHJ1 ZTsKKyAgICBib29sIGV4ZWN1dGFibGUgPSBmYWxzZTsKKyAgICBPU0FsbG9jYXRvcjo6Y29tbWl0 KHN0YXJ0QWRkcmVzcywgc2l6ZUluQnl0ZXMsIHdyaXRhYmxlLCBleGVjdXRhYmxlKTsKKyNlbHNl CisgICAgYm9vbCB3cml0YWJsZSA9IHRydWU7CisgICAgYm9vbCBleGVjdXRhYmxlID0gZmFsc2U7 CisgICAgT1NBbGxvY2F0b3I6OmNvbW1pdChzdGFydEFkZHJlc3MsIHNpemVJbkJ5dGVzLCB3cml0 YWJsZSwgZXhlY3V0YWJsZSk7CisgICAgbWVtc2V0KHN0YXJ0QWRkcmVzcywgMCwgc2l6ZUluQnl0 ZXMpOworI2VuZGlmCit9CisKIFJlZlB0cjxNZW1vcnk+IE1lbW9yeTo6Y3JlYXRlKFZNJiB2bSwg UGFnZUNvdW50IGluaXRpYWwsIFBhZ2VDb3VudCBtYXhpbXVtKQogewogICAgIEFTU0VSVChpbml0 aWFsKTsKQEAgLTI5MywxNiArMzEwLDE0IEBAIFJlZlB0cjxNZW1vcnk+IE1lbW9yeTo6Y3JlYXRl KFZNJiB2bSwgUGFnZUNvdW50IGluaXRpYWwsIFBhZ2VDb3VudCBtYXhpbXVtKQogICAgIH0KICAg ICAKICAgICBpZiAoZmFzdE1lbW9yeSkgewotICAgICAgICBib29sIHdyaXRhYmxlID0gdHJ1ZTsK LSAgICAgICAgYm9vbCBleGVjdXRhYmxlID0gZmFsc2U7Ci0gICAgICAgIE9TQWxsb2NhdG9yOjpj b21taXQoZmFzdE1lbW9yeSwgaW5pdGlhbEJ5dGVzLCB3cml0YWJsZSwgZXhlY3V0YWJsZSk7CiAg ICAgICAgIAogICAgICAgICBpZiAobXByb3RlY3QoZmFzdE1lbW9yeSArIGluaXRpYWxCeXRlcywg TWVtb3J5OjpmYXN0TWFwcGVkQnl0ZXMoKSAtIGluaXRpYWxCeXRlcywgUFJPVF9OT05FKSkgewog ICAgICAgICAgICAgZGF0YUxvZygibXByb3RlY3QgZmFpbGVkOiAiLCBzdHJlcnJvcihlcnJubyks ICJcbiIpOwogICAgICAgICAgICAgUkVMRUFTRV9BU1NFUlRfTk9UX1JFQUNIRUQoKTsKICAgICAg ICAgfQorCisgICAgICAgIGNvbW1pdFplcm9QYWdlcyhmYXN0TWVtb3J5LCBpbml0aWFsQnl0ZXMp OwogICAgICAgICAKLSAgICAgICAgbWVtc2V0KGZhc3RNZW1vcnksIDAsIGluaXRpYWxCeXRlcyk7 CiAgICAgICAgIHJldHVybiBhZG9wdFJlZihuZXcgTWVtb3J5KGZhc3RNZW1vcnksIGluaXRpYWws IG1heGltdW0sIE1lbW9yeTo6ZmFzdE1hcHBlZEJ5dGVzKCksIE1lbW9yeU1vZGU6OlNpZ25hbGlu ZykpOwogICAgIH0KICAgICAKQEAgLTQwMCw3ICs0MTUsNyBAQCBib29sIE1lbW9yeTo6Z3JvdyhW TSYgdm0sIFBhZ2VDb3VudCBuZXdTaXplKQogICAgICAgICAgICAgZGF0YUxvZ0xuSWYodmVyYm9z ZSwgIk1lbW9yeTo6Z3JvdyBpbi1wbGFjZSBmYWlsZWQgIiwgKnRoaXMpOwogICAgICAgICAgICAg cmV0dXJuIGZhbHNlOwogICAgICAgICB9Ci0gICAgICAgIG1lbXNldChzdGFydEFkZHJlc3MsIDAs IGV4dHJhQnl0ZXMpOworICAgICAgICBjb21taXRaZXJvUGFnZXMoc3RhcnRBZGRyZXNzLCBleHRy YUJ5dGVzKTsKICAgICAgICAgbV9zaXplID0gZGVzaXJlZFNpemU7CiAgICAgICAgIHJldHVybiB0 cnVlOwogICAgIH0gfQo= 317375 2017-08-06 12:09:59 -0700 2017-08-06 23:46:54 -0700 Patch bug-175150-20170807040958.patch text/plain 4657 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjIwMzI0CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAw ZTIzZWYyZWM3YzAwOWMxYzhjNzMyNDgyZDJmOTI5NDIxMWI3ZmZiLi4xNjFkZmNiZjMyMTU0NDI2 MjVlYWQyYWQ2OGQ5MTZhMzViN2FlNjk1IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs NSArMSwyNCBAQAogMjAxNy0wOC0wNiAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KIAorICAgICAgICBbTGludXhdIENsZWFyIFdhc21NZW1vcnkgd2l0aCBtYWR2aWNlIGlu c3RlYWQgb2YgbWVtc2V0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0xNzUxNTAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBJbiBMaW51eCwgemVyb2luZyBwYWdlcyB3aXRoIG1lbXNldCBwb3B1bGF0ZXMgYmFj a2luZyBzdG9yZS4KKyAgICAgICAgSW5zdGVhZCwgd2Ugc2hvdWxkIHVzZSBtYWR2aXNlIHdpdGgg TUFEVl9ET05UTkVFRC4gSXQgZGlzY2FyZHMKKyAgICAgICAgcGFnZXMuIEFuZCBpZiB5b3UgYWNj ZXNzIHRoZXNlIHBhZ2VzLCBvbi1kZW1hbmQtemVyby1wYWdlcyB3aWxsCisgICAgICAgIGJlIHNo b3duLgorCisgICAgICAgIFdlIGFsc28gY29tbWl0IGdyb3duIHBhZ2VzIGluIGFsbCBPU2VzLgor CisgICAgICAgICogd2FzbS9XYXNtTWVtb3J5LmNwcDoKKyAgICAgICAgKEpTQzo6V2FzbTo6Y29t bWl0WmVyb1BhZ2VzKToKKyAgICAgICAgKEpTQzo6V2FzbTo6TWVtb3J5OjpjcmVhdGUpOgorICAg ICAgICAoSlNDOjpXYXNtOjpNZW1vcnk6Omdyb3cpOgorCisyMDE3LTA4LTA2ICBZdXN1a2UgU3V6 dWtpICA8dXRhdGFuZS50ZWFAZ21haWwuY29tPgorCiAgICAgICAgIFByb21pc2UgcmVzb2x2ZSBh bmQgcmVqZWN0IGZ1bmN0aW9uIHNob3VsZCBoYXZlIGxlbmd0aCA9IDEKICAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3NTI0MgogCmRpZmYgLS1naXQgYS9T b3VyY2UvSmF2YVNjcmlwdENvcmUvd2FzbS9XYXNtTWVtb3J5LmNwcCBiL1NvdXJjZS9KYXZhU2Ny aXB0Q29yZS93YXNtL1dhc21NZW1vcnkuY3BwCmluZGV4IGRiNzczMWY1N2ZlYTg1NzNhMjYwMWNm Y2Y3YTMyZDI1MmViZTVkZjQuLmFlMjkzMTY2MjE0MDAyMjVmZWM0NWUwZDkzZDczNjMwNjQyZWRj OWQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93YXNtL1dhc21NZW1vcnkuY3Bw CisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93YXNtL1dhc21NZW1vcnkuY3BwCkBAIC0yNTUs NiArMjU1LDIzIEBAIE1lbW9yeTo6TWVtb3J5KHZvaWQqIG1lbW9yeSwgUGFnZUNvdW50IGluaXRp YWwsIFBhZ2VDb3VudCBtYXhpbXVtLCBzaXplX3QgbWFwcGVkCiAgICAgZGF0YUxvZ0xuSWYodmVy Ym9zZSwgIk1lbW9yeTo6TWVtb3J5IGFsbG9jYXRpbmcgIiwgKnRoaXMpOwogfQogCitzdGF0aWMg dm9pZCBjb21taXRaZXJvUGFnZXModm9pZCogc3RhcnRBZGRyZXNzLCBzaXplX3Qgc2l6ZUluQnl0 ZXMpCit7CisjaWYgT1MoTElOVVgpCisgICAgLy8gSW4gTGludXgsIE1BRFZfRE9OVE5FRUQgY2xl YXJzIGJhY2tpbmcgcGFnZXMgd2l0aCB6ZXJvLiBCZSBDYXJlZnVsIHRoYXQgTUFEVl9ET05UTkVF RCBzaG93cyBkaWZmZXJlbnQgc2VtYW50aWNzIGluIGRpZmZlcmVudCBPU2VzLgorICAgIC8vIEZv ciBleGFtcGxlLCBGcmVlQlNEIGRvZXMgbm90IGNsZWFyIGJhY2tpbmcgcGFnZXMgaW1tZWRpYXRl bHkuCisgICAgd2hpbGUgKG1hZHZpc2Uoc3RhcnRBZGRyZXNzLCBzaXplSW5CeXRlcywgTUFEVl9E T05UTkVFRCkgPT0gLTEgJiYgZXJybm8gPT0gRUFHQUlOKSB7IH0KKyAgICBib29sIHdyaXRhYmxl ID0gdHJ1ZTsKKyAgICBib29sIGV4ZWN1dGFibGUgPSBmYWxzZTsKKyAgICBPU0FsbG9jYXRvcjo6 Y29tbWl0KHN0YXJ0QWRkcmVzcywgc2l6ZUluQnl0ZXMsIHdyaXRhYmxlLCBleGVjdXRhYmxlKTsK KyNlbHNlCisgICAgYm9vbCB3cml0YWJsZSA9IHRydWU7CisgICAgYm9vbCBleGVjdXRhYmxlID0g ZmFsc2U7CisgICAgT1NBbGxvY2F0b3I6OmNvbW1pdChzdGFydEFkZHJlc3MsIHNpemVJbkJ5dGVz LCB3cml0YWJsZSwgZXhlY3V0YWJsZSk7CisgICAgbWVtc2V0KHN0YXJ0QWRkcmVzcywgMCwgc2l6 ZUluQnl0ZXMpOworI2VuZGlmCit9CisKIFJlZlB0cjxNZW1vcnk+IE1lbW9yeTo6Y3JlYXRlKFZN JiB2bSwgUGFnZUNvdW50IGluaXRpYWwsIFBhZ2VDb3VudCBtYXhpbXVtKQogewogICAgIEFTU0VS VChpbml0aWFsKTsKQEAgLTI5MywxNiArMzEwLDE0IEBAIFJlZlB0cjxNZW1vcnk+IE1lbW9yeTo6 Y3JlYXRlKFZNJiB2bSwgUGFnZUNvdW50IGluaXRpYWwsIFBhZ2VDb3VudCBtYXhpbXVtKQogICAg IH0KICAgICAKICAgICBpZiAoZmFzdE1lbW9yeSkgewotICAgICAgICBib29sIHdyaXRhYmxlID0g dHJ1ZTsKLSAgICAgICAgYm9vbCBleGVjdXRhYmxlID0gZmFsc2U7Ci0gICAgICAgIE9TQWxsb2Nh dG9yOjpjb21taXQoZmFzdE1lbW9yeSwgaW5pdGlhbEJ5dGVzLCB3cml0YWJsZSwgZXhlY3V0YWJs ZSk7CiAgICAgICAgIAogICAgICAgICBpZiAobXByb3RlY3QoZmFzdE1lbW9yeSArIGluaXRpYWxC eXRlcywgTWVtb3J5OjpmYXN0TWFwcGVkQnl0ZXMoKSAtIGluaXRpYWxCeXRlcywgUFJPVF9OT05F KSkgewogICAgICAgICAgICAgZGF0YUxvZygibXByb3RlY3QgZmFpbGVkOiAiLCBzdHJlcnJvcihl cnJubyksICJcbiIpOwogICAgICAgICAgICAgUkVMRUFTRV9BU1NFUlRfTk9UX1JFQUNIRUQoKTsK ICAgICAgICAgfQorCisgICAgICAgIGNvbW1pdFplcm9QYWdlcyhmYXN0TWVtb3J5LCBpbml0aWFs Qnl0ZXMpOwogICAgICAgICAKLSAgICAgICAgbWVtc2V0KGZhc3RNZW1vcnksIDAsIGluaXRpYWxC eXRlcyk7CiAgICAgICAgIHJldHVybiBhZG9wdFJlZihuZXcgTWVtb3J5KGZhc3RNZW1vcnksIGlu aXRpYWwsIG1heGltdW0sIE1lbW9yeTo6ZmFzdE1hcHBlZEJ5dGVzKCksIE1lbW9yeU1vZGU6OlNp Z25hbGluZykpOwogICAgIH0KICAgICAKQEAgLTQwMCw3ICs0MTUsNyBAQCBib29sIE1lbW9yeTo6 Z3JvdyhWTSYgdm0sIFBhZ2VDb3VudCBuZXdTaXplKQogICAgICAgICAgICAgZGF0YUxvZ0xuSWYo dmVyYm9zZSwgIk1lbW9yeTo6Z3JvdyBpbi1wbGFjZSBmYWlsZWQgIiwgKnRoaXMpOwogICAgICAg ICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICB9Ci0gICAgICAgIG1lbXNldChzdGFydEFkZHJl c3MsIDAsIGV4dHJhQnl0ZXMpOworICAgICAgICBjb21taXRaZXJvUGFnZXMoc3RhcnRBZGRyZXNz LCBleHRyYUJ5dGVzKTsKICAgICAgICAgbV9zaXplID0gZGVzaXJlZFNpemU7CiAgICAgICAgIHJl dHVybiB0cnVlOwogICAgIH0gfQpkaWZmIC0tZ2l0IGEvSlNUZXN0cy9DaGFuZ2VMb2cgYi9KU1Rl c3RzL0NoYW5nZUxvZwppbmRleCBlZTMyZDEzOTc1NmY4ZjA2NDE0YmEyYjM5YWE3N2ZjMzU3MzFj MjlhLi4zNWQxNDllYmY1N2I4YjBlYjczMWY5YmY3MzY3ZGUzMmViNDMzZWVmIDEwMDY0NAotLS0g YS9KU1Rlc3RzL0NoYW5nZUxvZworKysgYi9KU1Rlc3RzL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE0 IEBACiAyMDE3LTA4LTA2ICBZdXN1a2UgU3V6dWtpICA8dXRhdGFuZS50ZWFAZ21haWwuY29tPgog CisgICAgICAgIFtMaW51eF0gQ2xlYXIgV2FzbU1lbW9yeSB3aXRoIG1hZHZpY2UgaW5zdGVhZCBv ZiBtZW1zZXQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTE3NTE1MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogd2FzbS9qcy1hcGkvdGVzdF9tZW1vcnlfY29uc3RydWN0b3IuanM6CisKKzIwMTctMDgtMDYg IFl1c3VrZSBTdXp1a2kgIDx1dGF0YW5lLnRlYUBnbWFpbC5jb20+CisKICAgICAgICAgUHJvbWlz ZSByZXNvbHZlIGFuZCByZWplY3QgZnVuY3Rpb24gc2hvdWxkIGhhdmUgbGVuZ3RoID0gMQogICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTc1MjQyCiAKZGlm ZiAtLWdpdCBhL0pTVGVzdHMvd2FzbS9qcy1hcGkvdGVzdF9tZW1vcnlfY29uc3RydWN0b3IuanMg Yi9KU1Rlc3RzL3dhc20vanMtYXBpL3Rlc3RfbWVtb3J5X2NvbnN0cnVjdG9yLmpzCmluZGV4IGI4 ODkzZWFiN2FmMmZhYzBlYTM0OGM1ZWU1MjU2N2MzY2U1NGM2ZDkuLmU2N2RmOTJhZGZiOTZjYmVi YmVjMWMxNGMzZGFmZjFlMmRkZTYxMGUgMTAwNjQ0Ci0tLSBhL0pTVGVzdHMvd2FzbS9qcy1hcGkv dGVzdF9tZW1vcnlfY29uc3RydWN0b3IuanMKKysrIGIvSlNUZXN0cy93YXNtL2pzLWFwaS90ZXN0 X21lbW9yeV9jb25zdHJ1Y3Rvci5qcwpAQCAtMSw1ICsxLDQgQEAKIC8vIEZJWE1FOiB1c2UgdGhl IGFzc2VydCBsaWJyYXJ5OiBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTY1Njg0Ci0vL0Agc2tpcCBpZiAkbWVtb3J5TGltaXRlZAogaW1wb3J0IEJ1aWxkZXIgZnJvbSAn Li4vQnVpbGRlci5qcyc7CiAKIGZ1bmN0aW9uIGFzc2VydChiKSB7Cg== 317404 2017-08-06 23:46:57 -0700 2017-08-07 10:18:33 -0700 Patch bug-175150-20170807154656.patch text/plain 3497 ysuzuki U3VidmVyc2lvbiBSZXZpc2lvbjogMjIwMzI0CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAw ZTIzZWYyZWM3YzAwOWMxYzhjNzMyNDgyZDJmOTI5NDIxMWI3ZmZiLi4xNjFkZmNiZjMyMTU0NDI2 MjVlYWQyYWQ2OGQ5MTZhMzViN2FlNjk1IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs NSArMSwyNCBAQAogMjAxNy0wOC0wNiAgWXVzdWtlIFN1enVraSAgPHV0YXRhbmUudGVhQGdtYWls LmNvbT4KIAorICAgICAgICBbTGludXhdIENsZWFyIFdhc21NZW1vcnkgd2l0aCBtYWR2aWNlIGlu c3RlYWQgb2YgbWVtc2V0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0xNzUxNTAKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBJbiBMaW51eCwgemVyb2luZyBwYWdlcyB3aXRoIG1lbXNldCBwb3B1bGF0ZXMgYmFj a2luZyBzdG9yZS4KKyAgICAgICAgSW5zdGVhZCwgd2Ugc2hvdWxkIHVzZSBtYWR2aXNlIHdpdGgg TUFEVl9ET05UTkVFRC4gSXQgZGlzY2FyZHMKKyAgICAgICAgcGFnZXMuIEFuZCBpZiB5b3UgYWNj ZXNzIHRoZXNlIHBhZ2VzLCBvbi1kZW1hbmQtemVyby1wYWdlcyB3aWxsCisgICAgICAgIGJlIHNo b3duLgorCisgICAgICAgIFdlIGFsc28gY29tbWl0IGdyb3duIHBhZ2VzIGluIGFsbCBPU2VzLgor CisgICAgICAgICogd2FzbS9XYXNtTWVtb3J5LmNwcDoKKyAgICAgICAgKEpTQzo6V2FzbTo6Y29t bWl0WmVyb1BhZ2VzKToKKyAgICAgICAgKEpTQzo6V2FzbTo6TWVtb3J5OjpjcmVhdGUpOgorICAg ICAgICAoSlNDOjpXYXNtOjpNZW1vcnk6Omdyb3cpOgorCisyMDE3LTA4LTA2ICBZdXN1a2UgU3V6 dWtpICA8dXRhdGFuZS50ZWFAZ21haWwuY29tPgorCiAgICAgICAgIFByb21pc2UgcmVzb2x2ZSBh bmQgcmVqZWN0IGZ1bmN0aW9uIHNob3VsZCBoYXZlIGxlbmd0aCA9IDEKICAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3NTI0MgogCmRpZmYgLS1naXQgYS9T b3VyY2UvSmF2YVNjcmlwdENvcmUvd2FzbS9XYXNtTWVtb3J5LmNwcCBiL1NvdXJjZS9KYXZhU2Ny aXB0Q29yZS93YXNtL1dhc21NZW1vcnkuY3BwCmluZGV4IGRiNzczMWY1N2ZlYTg1NzNhMjYwMWNm Y2Y3YTMyZDI1MmViZTVkZjQuLmFlMjkzMTY2MjE0MDAyMjVmZWM0NWUwZDkzZDczNjMwNjQyZWRj OWQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93YXNtL1dhc21NZW1vcnkuY3Bw CisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93YXNtL1dhc21NZW1vcnkuY3BwCkBAIC0yNTUs NiArMjU1LDIzIEBAIE1lbW9yeTo6TWVtb3J5KHZvaWQqIG1lbW9yeSwgUGFnZUNvdW50IGluaXRp YWwsIFBhZ2VDb3VudCBtYXhpbXVtLCBzaXplX3QgbWFwcGVkCiAgICAgZGF0YUxvZ0xuSWYodmVy Ym9zZSwgIk1lbW9yeTo6TWVtb3J5IGFsbG9jYXRpbmcgIiwgKnRoaXMpOwogfQogCitzdGF0aWMg dm9pZCBjb21taXRaZXJvUGFnZXModm9pZCogc3RhcnRBZGRyZXNzLCBzaXplX3Qgc2l6ZUluQnl0 ZXMpCit7CisjaWYgT1MoTElOVVgpCisgICAgLy8gSW4gTGludXgsIE1BRFZfRE9OVE5FRUQgY2xl YXJzIGJhY2tpbmcgcGFnZXMgd2l0aCB6ZXJvLiBCZSBDYXJlZnVsIHRoYXQgTUFEVl9ET05UTkVF RCBzaG93cyBkaWZmZXJlbnQgc2VtYW50aWNzIGluIGRpZmZlcmVudCBPU2VzLgorICAgIC8vIEZv ciBleGFtcGxlLCBGcmVlQlNEIGRvZXMgbm90IGNsZWFyIGJhY2tpbmcgcGFnZXMgaW1tZWRpYXRl bHkuCisgICAgd2hpbGUgKG1hZHZpc2Uoc3RhcnRBZGRyZXNzLCBzaXplSW5CeXRlcywgTUFEVl9E T05UTkVFRCkgPT0gLTEgJiYgZXJybm8gPT0gRUFHQUlOKSB7IH0KKyAgICBib29sIHdyaXRhYmxl ID0gdHJ1ZTsKKyAgICBib29sIGV4ZWN1dGFibGUgPSBmYWxzZTsKKyAgICBPU0FsbG9jYXRvcjo6 Y29tbWl0KHN0YXJ0QWRkcmVzcywgc2l6ZUluQnl0ZXMsIHdyaXRhYmxlLCBleGVjdXRhYmxlKTsK KyNlbHNlCisgICAgYm9vbCB3cml0YWJsZSA9IHRydWU7CisgICAgYm9vbCBleGVjdXRhYmxlID0g ZmFsc2U7CisgICAgT1NBbGxvY2F0b3I6OmNvbW1pdChzdGFydEFkZHJlc3MsIHNpemVJbkJ5dGVz LCB3cml0YWJsZSwgZXhlY3V0YWJsZSk7CisgICAgbWVtc2V0KHN0YXJ0QWRkcmVzcywgMCwgc2l6 ZUluQnl0ZXMpOworI2VuZGlmCit9CisKIFJlZlB0cjxNZW1vcnk+IE1lbW9yeTo6Y3JlYXRlKFZN JiB2bSwgUGFnZUNvdW50IGluaXRpYWwsIFBhZ2VDb3VudCBtYXhpbXVtKQogewogICAgIEFTU0VS VChpbml0aWFsKTsKQEAgLTI5MywxNiArMzEwLDE0IEBAIFJlZlB0cjxNZW1vcnk+IE1lbW9yeTo6 Y3JlYXRlKFZNJiB2bSwgUGFnZUNvdW50IGluaXRpYWwsIFBhZ2VDb3VudCBtYXhpbXVtKQogICAg IH0KICAgICAKICAgICBpZiAoZmFzdE1lbW9yeSkgewotICAgICAgICBib29sIHdyaXRhYmxlID0g dHJ1ZTsKLSAgICAgICAgYm9vbCBleGVjdXRhYmxlID0gZmFsc2U7Ci0gICAgICAgIE9TQWxsb2Nh dG9yOjpjb21taXQoZmFzdE1lbW9yeSwgaW5pdGlhbEJ5dGVzLCB3cml0YWJsZSwgZXhlY3V0YWJs ZSk7CiAgICAgICAgIAogICAgICAgICBpZiAobXByb3RlY3QoZmFzdE1lbW9yeSArIGluaXRpYWxC eXRlcywgTWVtb3J5OjpmYXN0TWFwcGVkQnl0ZXMoKSAtIGluaXRpYWxCeXRlcywgUFJPVF9OT05F KSkgewogICAgICAgICAgICAgZGF0YUxvZygibXByb3RlY3QgZmFpbGVkOiAiLCBzdHJlcnJvcihl cnJubyksICJcbiIpOwogICAgICAgICAgICAgUkVMRUFTRV9BU1NFUlRfTk9UX1JFQUNIRUQoKTsK ICAgICAgICAgfQorCisgICAgICAgIGNvbW1pdFplcm9QYWdlcyhmYXN0TWVtb3J5LCBpbml0aWFs Qnl0ZXMpOwogICAgICAgICAKLSAgICAgICAgbWVtc2V0KGZhc3RNZW1vcnksIDAsIGluaXRpYWxC eXRlcyk7CiAgICAgICAgIHJldHVybiBhZG9wdFJlZihuZXcgTWVtb3J5KGZhc3RNZW1vcnksIGlu aXRpYWwsIG1heGltdW0sIE1lbW9yeTo6ZmFzdE1hcHBlZEJ5dGVzKCksIE1lbW9yeU1vZGU6OlNp Z25hbGluZykpOwogICAgIH0KICAgICAKQEAgLTQwMCw3ICs0MTUsNyBAQCBib29sIE1lbW9yeTo6 Z3JvdyhWTSYgdm0sIFBhZ2VDb3VudCBuZXdTaXplKQogICAgICAgICAgICAgZGF0YUxvZ0xuSWYo dmVyYm9zZSwgIk1lbW9yeTo6Z3JvdyBpbi1wbGFjZSBmYWlsZWQgIiwgKnRoaXMpOwogICAgICAg ICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICB9Ci0gICAgICAgIG1lbXNldChzdGFydEFkZHJl c3MsIDAsIGV4dHJhQnl0ZXMpOworICAgICAgICBjb21taXRaZXJvUGFnZXMoc3RhcnRBZGRyZXNz LCBleHRyYUJ5dGVzKTsKICAgICAgICAgbV9zaXplID0gZGVzaXJlZFNpemU7CiAgICAgICAgIHJl dHVybiB0cnVlOwogICAgIH0gfQo=