| Summary: | DelayedReleaseScope drops locks during GC which can cause a thread switch and code reentry | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||||||||
| Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | commit-queue, ggaren, mark.lam, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Attachments: |
|
||||||||||||||
Created attachment 246072 [details]
Patch
Comment on attachment 246072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246072&action=review I've provided some feedback for now but I'm not sold on this solution. The main issue I see is that we used to release all the delayed releases at the end of each GC cycle. Now, with this patch, we'll have to wait till the outermost JSLock is unlocked. If a piece of JS code is long running (e.g. the benchmarks), it will continue to retain these DelayedReleasedObjects for a long time even if we've GC'ed several times. As a result, the runtime memory profile will higher peak usage. From your bug description, I think the issue at hand is that another thread can see an null UnlinkedCodeBlock in the Cache. The fact that we drop all locks may have enabled the issue to manifest, but in my understanding is not core to the issue. Instead, would be possible to fix the issue by changing CodeCache to: 1. Instead of add the SourceCode immediately, do a find to see if an UnlinkedCodeBlock for it exists. 2. If it exists, use the cached UnlinkedCodeBlock. 3. If not: 3.1 go ahead and parse the SourceCode and make the UnlinkedCodeBlock. 3.2. Check again is an UnlinkedCodeBlock has already been added for this SourceCode. 3.3. If not already present, add the UnlinkedCodeBlock to the cache. 3.4. If already present, release the new UnlinkedCodeBlock, and use the one in the cache. Theoretically, this might result in some wasted work where the same SourceCode being parsed into an UnlinkedCodeBlock twice. However, in practice, if we don't have 2 threads, that will never happen. Even with running on 2 threads, the collision mwill probably still be rare. Would that work? > Source/JavaScriptCore/ChangeLog:7 > + Your ChangeLog comment describes what was done but not why. I think your description of the issue in bugzilla did a good job of explaining what the issue is. Would you mind adding it here before you start explaining how it is fixed? > Source/JavaScriptCore/ChangeLog:17 > + by calling into JS. This case is acually tested by testapi.mm. typo: "acually". However, I think "already" would be a better fit here. > Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:1758 > -</Project> > \ No newline at end of file > +</Project> Is this necessary and ok to do? > Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:4390 > -</Project> > \ No newline at end of file > +</Project> Ditto. > Source/JavaScriptCore/heap/Heap.h:118 > + JS_EXPORT_PRIVATE void releaseDelayedReleasedObjects(); No need to JS_EXPORT_PRIVATE this function as it is only used inside JSC. (In reply to comment #2) > Comment on attachment 246072 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=246072&action=review > > I've provided some feedback for now but I'm not sold on this solution. The > main issue I see is that we used to release all the delayed releases at the > end of each GC cycle. Now, with this patch, we'll have to wait till the > outermost JSLock is unlocked. If a piece of JS code is long running (e.g. > the benchmarks), it will continue to retain these DelayedReleasedObjects for > a long time even if we've GC'ed several times. As a result, the runtime > memory profile will higher peak usage. I don't think any of our benchmarks use the ObjC API or retain ObjC objects. > From your bug description, I think the issue at hand is that another thread > can see an null UnlinkedCodeBlock in the Cache. The fact that we drop all > locks may have enabled the issue to manifest, but in my understanding is not > core to the issue. Instead, would be possible to fix the issue by changing > CodeCache to: > > 1. Instead of add the SourceCode immediately, do a find to see if an > UnlinkedCodeBlock for it exists. > 2. If it exists, use the cached UnlinkedCodeBlock. > 3. If not: > 3.1 go ahead and parse the SourceCode and make the UnlinkedCodeBlock. > 3.2. Check again is an UnlinkedCodeBlock has already been added for this > SourceCode. > 3.3. If not already present, add the UnlinkedCodeBlock to the cache. > 3.4. If already present, release the new UnlinkedCodeBlock, and use the > one in the cache. > > Theoretically, this might result in some wasted work where the same > SourceCode being parsed into an UnlinkedCodeBlock twice. However, in > practice, if we don't have 2 threads, that will never happen. Even with > running on 2 threads, the collision mwill probably still be rare. > > Would that work? I implemented a trial solution in CodeCache:add() similar to what you describe and it worked fine for this issue. I discussed it with Geoff and he said the problem with that solution is there are many other places that we do this "add null and fill in the value later" kind of HashMap accesses. From that discussion, we agreed on this approach of not dropping the lock during GC and releasing delayed objects after the lock was dropped exiting the VM. > > Source/JavaScriptCore/ChangeLog:7 > > + > > Your ChangeLog comment describes what was done but not why. I think your > description of the issue in bugzilla did a good job of explaining what the > issue is. Would you mind adding it here before you start explaining how it > is fixed? I added this paragraph to the top of the ChangeLog entry: The root issue is that one thread, while allocating JS objects may need to garbage collect. During that garbage collection, if there are delayed Objective C objects that need to be released, we drop the locks to release them. While the locks are release by that thread, another thread can enter the VM and might have exactly the same source and therefore hit this issue. This fixes the problem by not dropping the locks during garbage collection. > > Source/JavaScriptCore/ChangeLog:17 > > + by calling into JS. This case is acually tested by testapi.mm. > > typo: "acually". However, I think "already" would be a better fit here. Made that fix locally. > > Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:1758 > > -</Project> > > \ No newline at end of file > > +</Project> > > Is this necessary and ok to do? Given that it is XML, I think it is fine. The Win EWS bot built with the patch just fine. > > Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:4390 > > -</Project> > > \ No newline at end of file > > +</Project> > > Ditto. > > > Source/JavaScriptCore/heap/Heap.h:118 > > + JS_EXPORT_PRIVATE void releaseDelayedReleasedObjects(); > > No need to JS_EXPORT_PRIVATE this function as it is only used inside JSC. Fixed. Here are benchmark results. Neutral to maybe a slight speed up.
Benchmark report for SunSpider, LongSpider, V8Spider, Octane, Kraken, JSRegress, AsmBench, and CompressionBench on msaboff-pro (MacPro5,1).
VMs tested:
"Baseline" at /Volumes/Data/src/webkit.baseline/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/Resources/jsc
"GCDontDropLocks" at /Volumes/Data/src/webkit/WebKitBuild/Release/JavaScriptCore.framework/Versions/A/Resources/jsc
Collected 4 samples per benchmark/VM, with 4 VM invocations per benchmark. Emitted a call to gc() between sample measurements.
Used 1 benchmark iteration per VM invocation for warm-up. Used the jsc-specific preciseTime() function to get microsecond-level
timing. Reporting benchmark execution times with 95% confidence intervals in milliseconds.
Baseline GCDontDropLocks
SunSpider:
3d-cube 8.2874+-0.4075 ? 8.4276+-0.1794 ? might be 1.0169x slower
3d-morph 12.6283+-0.2559 12.6267+-0.3875
3d-raytrace 11.0775+-0.3408 10.9921+-0.3171
access-binary-trees 3.5512+-0.2837 3.4613+-0.2011 might be 1.0260x faster
access-fannkuch 10.4371+-0.3147 ? 10.5487+-0.1167 ? might be 1.0107x slower
access-nbody 5.6537+-0.0808 5.6317+-0.2477
access-nsieve 5.7880+-0.2364 ? 6.1447+-0.7727 ? might be 1.0616x slower
bitops-3bit-bits-in-byte 2.3798+-0.1259 2.3016+-0.1406 might be 1.0340x faster
bitops-bits-in-byte 6.4005+-0.1407 ? 6.6443+-0.5729 ? might be 1.0381x slower
bitops-bitwise-and 3.1192+-0.1480 ? 3.1356+-0.0724 ?
bitops-nsieve-bits 6.8399+-0.3972 ? 6.8992+-0.0346 ?
controlflow-recursive 3.6331+-0.2296 ? 3.6355+-0.3157 ?
crypto-aes 7.1508+-0.4122 ? 7.1888+-0.4024 ?
crypto-md5 4.3215+-0.2786 4.3129+-0.5220
crypto-sha1 4.3408+-0.1204 4.1880+-0.2596 might be 1.0365x faster
date-format-tofte 16.7717+-0.7366 16.6817+-0.2536
date-format-xparb 9.4168+-0.5794 ? 9.5230+-0.4646 ? might be 1.0113x slower
math-cordic 5.6259+-0.2124 5.6171+-0.3203
math-partial-sums 11.0132+-0.1232 ? 11.0358+-0.2127 ?
math-spectral-norm 3.6334+-0.1861 3.5872+-0.0561 might be 1.0129x faster
regexp-dna 11.6884+-0.5060 11.3149+-0.2350 might be 1.0330x faster
string-base64 7.2542+-0.3255 7.2304+-0.3460
string-fasta 12.2562+-0.3317 ? 12.3046+-0.4177 ?
string-tagcloud 17.7007+-0.3883 17.3237+-0.3029 might be 1.0218x faster
string-unpack-code 37.3306+-1.0074 ? 38.2923+-1.0488 ? might be 1.0258x slower
string-validate-input 8.5934+-0.3738 8.4857+-0.1985 might be 1.0127x faster
<arithmetic> * 9.1113+-0.0862 ? 9.1360+-0.1193 ? might be 1.0027x slower
<geometric> 7.4436+-0.0882 7.4396+-0.1216 might be 1.0005x faster
<harmonic> 6.2588+-0.1045 6.2312+-0.1153 might be 1.0044x faster
Baseline GCDontDropLocks
LongSpider:
3d-cube 1600.9268+-29.6274 1597.9611+-28.7411
3d-morph 1980.2997+-4.5007 1979.3760+-1.9151
3d-raytrace 1563.5179+-9.3382 1561.7790+-9.3141
access-binary-trees 1870.8630+-11.6636 1853.6997+-8.9547
access-fannkuch 598.4971+-44.1336 ? 600.7025+-29.9472 ?
access-nbody 1486.9128+-8.4505 ? 1490.3475+-11.1716 ?
access-nsieve 1814.2460+-11.8846 ? 1819.5962+-20.7864 ?
bitops-3bit-bits-in-byte 71.4328+-0.9201 71.0125+-1.0927
bitops-bits-in-byte 369.8054+-11.8523 ? 373.0720+-14.0135 ?
bitops-nsieve-bits 1453.0237+-16.2679 ? 1453.2216+-11.1583 ?
controlflow-recursive 1089.6410+-3.6694 ? 1099.3997+-8.3668 ?
crypto-aes 1297.5742+-8.4535 1293.8137+-5.1115
crypto-md5 1083.3359+-11.6202 1082.2216+-9.4268
crypto-sha1 1449.4463+-2.9528 ? 1450.9452+-9.7966 ?
date-format-tofte 1514.6633+-84.9346 ? 1526.6085+-25.2648 ?
date-format-xparb 1423.4734+-27.9681 1388.8990+-28.5513 might be 1.0249x faster
math-cordic 1051.2505+-12.7771 1048.5900+-9.7412
math-partial-sums 1285.7495+-2.8893 1285.5657+-5.8522
math-spectral-norm 1218.3607+-1.6995 ? 1218.5185+-2.5982 ?
string-base64 635.0610+-1.5039 634.2584+-2.8186
string-fasta 873.4131+-26.1124 854.9326+-9.9061 might be 1.0216x faster
string-tagcloud 419.6511+-2.9527 417.9655+-4.1416
<arithmetic> 1188.6884+-7.9262 1186.4767+-3.6740 might be 1.0019x faster
<geometric> * 1002.3436+-7.7248 1000.5258+-2.7055 might be 1.0018x faster
<harmonic> 630.1777+-5.3515 628.3575+-3.6252 might be 1.0029x faster
Baseline GCDontDropLocks
V8Spider:
crypto 100.2801+-1.2653 ? 100.5043+-1.1308 ?
deltablue 126.9694+-5.3371 121.9311+-2.6027 might be 1.0413x faster
earley-boyer 83.0057+-5.4756 80.6595+-0.2354 might be 1.0291x faster
raytrace 51.3168+-0.9058 50.6696+-1.0394 might be 1.0128x faster
regexp 128.9222+-1.4115 128.8653+-0.9100
richards 132.2358+-2.4264 ? 133.1490+-1.0884 ?
splay 59.2132+-1.3879 ? 61.7690+-6.8158 ? might be 1.0432x slower
<arithmetic> 97.4205+-0.5011 96.7925+-0.9790 might be 1.0065x faster
<geometric> * 91.7400+-0.5463 91.3272+-1.5237 might be 1.0045x faster
<harmonic> 85.7713+-0.7367 85.5814+-2.1441 might be 1.0022x faster
Baseline GCDontDropLocks
Octane:
encrypt 0.46692+-0.01103 0.46354+-0.01443
decrypt 8.24243+-0.03055 8.23960+-0.01946
deltablue x2 0.39210+-0.00701 ? 0.39272+-0.00444 ?
earley 1.34903+-0.01789 1.33857+-0.03040
boyer 12.15227+-0.23771 11.99703+-0.02912 might be 1.0129x faster
navier-stokes x2 7.37743+-0.00696 ? 7.38730+-0.02524 ?
raytrace x2 2.67632+-0.33980 2.65387+-0.20697
richards x2 0.24530+-0.00471 0.24205+-0.00553 might be 1.0134x faster
splay x2 0.66180+-0.02769 0.65440+-0.00381 might be 1.0113x faster
regexp x2 63.30892+-0.65007 63.24368+-2.23599
pdfjs x2 101.20690+-1.02071 100.00601+-1.28065 might be 1.0120x faster
mandreel x2 106.90159+-1.56042 ? 107.15199+-0.70171 ?
gbemu x2 100.00511+-4.51717 99.72006+-3.37979
closure 0.98703+-0.01117 ? 0.98851+-0.01114 ?
jquery 12.52820+-0.14516 ? 12.63393+-0.15504 ?
box2d x2 28.81709+-0.42253 ^ 28.35351+-0.02981 ^ definitely 1.0163x faster
zlib x2 874.67743+-78.77564 ? 900.54356+-5.96773 ? might be 1.0296x slower
typescript x2 1473.44904+-44.86878 ? 1497.86053+-10.57159 ? might be 1.0166x slower
<arithmetic> 185.17213+-8.15047 ? 188.40269+-0.82534 ? might be 1.0174x slower
<geometric> * 13.48771+-0.19672 13.46913+-0.08889 might be 1.0014x faster
<harmonic> 1.38607+-0.00685 1.37592+-0.01121 might be 1.0074x faster
Baseline GCDontDropLocks
Kraken:
ai-astar 831.922+-10.318 818.597+-21.629 might be 1.0163x faster
audio-beat-detection 221.653+-5.461 221.273+-1.316
audio-dft 295.661+-2.959 ? 302.912+-19.833 ? might be 1.0245x slower
audio-fft 155.251+-0.618 ? 155.711+-0.351 ?
audio-oscillator 438.332+-3.393 436.554+-1.882
imaging-darkroom 334.565+-1.615 ? 335.556+-2.214 ?
imaging-desaturate 144.874+-0.283 ? 145.078+-0.613 ?
imaging-gaussian-blur 216.790+-0.959 216.398+-0.386
json-parse-financial 92.422+-1.096 92.126+-0.611
json-stringify-tinderbox 117.408+-11.631 111.417+-0.557 might be 1.0538x faster
stanford-crypto-aes 118.359+-1.730 ? 118.932+-2.909 ?
stanford-crypto-ccm 100.800+-12.170 ? 105.620+-20.493 ? might be 1.0478x slower
stanford-crypto-pbkdf2 315.251+-5.126 ? 317.996+-4.245 ?
stanford-crypto-sha256-iterative 95.067+-0.568 ? 95.376+-2.879 ?
<arithmetic> * 248.454+-1.219 248.110+-2.814 might be 1.0014x faster
<geometric> 198.810+-1.670 ? 198.998+-3.135 ? might be 1.0009x slower
<harmonic> 167.095+-2.223 ? 167.292+-4.056 ? might be 1.0012x slower
Baseline GCDontDropLocks
JSRegress:
abs-boolean 4.9039+-0.7798 4.6655+-0.1388 might be 1.0511x faster
adapt-to-double-divide 19.7751+-0.1035 19.7607+-0.1161
aliased-arguments-getbyval 1.4850+-0.0498 ? 1.4988+-0.0874 ?
allocate-big-object 3.9099+-0.1903 ? 3.9507+-0.6173 ? might be 1.0104x slower
arity-mismatch-inlining 1.3118+-0.1009 ? 1.3835+-0.0940 ? might be 1.0546x slower
array-access-polymorphic-structure 12.6620+-0.3088 ? 12.6750+-0.2490 ?
array-nonarray-polymorhpic-access 68.9453+-0.3822 ? 69.4298+-1.2578 ?
array-prototype-every 187.5782+-8.9778 184.9826+-4.2053 might be 1.0140x faster
array-prototype-forEach 183.1628+-7.2041 182.8010+-1.7654
array-prototype-map 224.4810+-6.0747 224.0670+-7.1083
array-prototype-some 189.9885+-2.2155 ? 191.6420+-6.1883 ?
array-splice-contiguous 95.0856+-4.1395 95.0295+-4.8040
array-with-double-add 7.4538+-0.1129 7.4410+-0.2286
array-with-double-increment 5.6599+-0.2689 5.5112+-0.2261 might be 1.0270x faster
array-with-double-mul-add 11.8201+-0.2235 11.6105+-0.1533 might be 1.0181x faster
array-with-double-sum 5.4011+-0.1249 ? 5.4792+-0.1940 ? might be 1.0145x slower
array-with-int32-add-sub 13.1190+-0.1629 13.0753+-0.0849
array-with-int32-or-double-sum 5.3206+-0.1428 ? 5.3509+-0.0814 ?
ArrayBuffer-DataView-alloc-large-long-lived
67.4449+-3.1558 65.0034+-1.2483 might be 1.0376x faster
ArrayBuffer-DataView-alloc-long-lived 28.3835+-2.6615 26.8492+-0.4847 might be 1.0571x faster
ArrayBuffer-Int32Array-byteOffset 5.1708+-0.1793 ? 5.3670+-0.2430 ? might be 1.0379x slower
ArrayBuffer-Int8Array-alloc-large-long-lived
69.3553+-1.3089 68.3317+-1.2618 might be 1.0150x faster
ArrayBuffer-Int8Array-alloc-long-lived-buffer
49.0463+-3.5148 45.8782+-5.1571 might be 1.0691x faster
ArrayBuffer-Int8Array-alloc-long-lived 25.9260+-1.2349 25.6655+-0.3735 might be 1.0102x faster
ArrayBuffer-Int8Array-alloc 21.1623+-0.4510 ? 21.2051+-0.5032 ?
asmjs_bool_bug 12.7102+-0.2582 ? 12.9255+-0.3227 ? might be 1.0169x slower
assign-custom-setter-polymorphic 6.0051+-0.2325 ? 6.1526+-0.2088 ? might be 1.0246x slower
assign-custom-setter 8.3787+-0.3240 8.1545+-0.0807 might be 1.0275x faster
basic-set 17.6423+-1.1215 17.5315+-0.9472
big-int-mul 7.7305+-0.1298 7.7203+-0.1079
boolean-test 4.9650+-0.0958 4.9219+-0.1432
branch-fold 5.2761+-0.1207 5.2139+-0.1087 might be 1.0119x faster
by-val-generic 15.6538+-0.4113 ? 15.8600+-0.7473 ? might be 1.0132x slower
call-spread-apply 25.7766+-0.3250 ? 26.1796+-1.0617 ? might be 1.0156x slower
call-spread-call 11.3736+-0.2834 ? 11.4882+-0.2004 ? might be 1.0101x slower
captured-assignments 0.8073+-0.1250 ? 0.8228+-0.1541 ? might be 1.0191x slower
cast-int-to-double 8.6315+-0.1372 ? 8.6738+-0.1119 ?
cell-argument 12.0446+-0.2477 ? 12.2531+-0.4238 ? might be 1.0173x slower
cfg-simplify 3.9150+-0.0722 ? 3.9724+-0.0507 ? might be 1.0147x slower
chain-getter-access 16.4993+-0.3234 16.4705+-0.2705
cmpeq-obj-to-obj-other 16.6860+-0.4798 ? 16.7102+-0.4701 ?
constant-test 8.0961+-0.0803 8.0090+-0.1410 might be 1.0109x faster
DataView-custom-properties 79.4473+-4.3748 76.1459+-3.1501 might be 1.0434x faster
delay-tear-off-arguments-strictmode 45.4232+-0.9265 ? 45.5005+-0.9970 ?
destructuring-arguments 9.1252+-0.1275 9.1111+-0.2100
destructuring-swap 8.3210+-0.1561 8.2637+-0.1086
direct-arguments-getbyval 1.5987+-0.3076 ? 1.6596+-0.3791 ? might be 1.0381x slower
div-boolean-double 5.8640+-0.0654 ? 5.9987+-0.1347 ? might be 1.0230x slower
div-boolean 10.2576+-0.0393 10.2510+-0.3121
double-get-by-val-out-of-bounds 8.1508+-0.2113 ? 8.2041+-0.2906 ?
double-pollution-getbyval 10.0820+-0.0961 ? 10.1068+-0.1696 ?
double-pollution-putbyoffset 8.0867+-0.1528 8.0607+-0.0867
double-to-int32-typed-array-no-inline 3.7795+-0.2651 3.7372+-0.0584 might be 1.0113x faster
double-to-int32-typed-array 3.2941+-0.1358 3.2575+-0.0923 might be 1.0112x faster
double-to-uint32-typed-array-no-inline 3.8181+-0.1766 ? 3.8832+-0.2454 ? might be 1.0170x slower
double-to-uint32-typed-array 3.3947+-0.1728 3.2814+-0.1806 might be 1.0345x faster
elidable-new-object-dag 62.6877+-2.1583 62.4510+-2.5799
elidable-new-object-roflcopter 242.1007+-5.5626 237.9272+-2.5464 might be 1.0175x faster
elidable-new-object-then-call 60.3154+-2.0250 58.7932+-0.9074 might be 1.0259x faster
elidable-new-object-tree 70.2939+-1.2846 ? 70.3883+-1.1415 ?
empty-string-plus-int 10.4761+-0.4592 10.2294+-0.2269 might be 1.0241x faster
emscripten-cube2hash 57.2521+-1.1727 ? 58.2951+-1.4164 ? might be 1.0182x slower
external-arguments-getbyval 2.5018+-0.1807 2.4797+-0.3049
external-arguments-putbyval 3.8517+-0.1520 3.7413+-0.2808 might be 1.0295x faster
fixed-typed-array-storage-var-index 2.0370+-0.0943 1.9715+-0.1770 might be 1.0332x faster
fixed-typed-array-storage 1.3853+-0.1255 ? 1.4150+-0.1272 ? might be 1.0215x slower
Float32Array-matrix-mult 8.4427+-0.6791 8.0106+-0.2693 might be 1.0539x faster
Float32Array-to-Float64Array-set 117.7137+-2.0385 ? 119.9370+-7.1261 ? might be 1.0189x slower
Float64Array-alloc-long-lived 104.3427+-0.7615 103.9674+-0.5007
Float64Array-to-Int16Array-set 147.8989+-5.7667 145.5093+-2.5894 might be 1.0164x faster
fold-double-to-int 24.5105+-0.5455 24.3843+-0.3457
fold-get-by-id-to-multi-get-by-offset-rare-int
14.3041+-0.5144 14.0502+-0.2147 might be 1.0181x faster
fold-get-by-id-to-multi-get-by-offset 12.2864+-0.3936 ? 12.4200+-0.5803 ? might be 1.0109x slower
fold-multi-get-by-offset-to-get-by-offset
10.5012+-1.4849 ? 11.6681+-0.8244 ? might be 1.1111x slower
fold-multi-get-by-offset-to-poly-get-by-offset
11.3296+-1.2084 ? 11.4252+-0.9003 ?
fold-multi-put-by-offset-to-poly-put-by-offset
10.5042+-1.1953 9.9715+-1.1096 might be 1.0534x faster
fold-multi-put-by-offset-to-put-by-offset
8.6668+-1.5594 ? 8.7687+-1.0436 ? might be 1.0118x slower
fold-multi-put-by-offset-to-replace-or-transition-put-by-offset
15.4473+-2.0953 ? 15.7321+-1.4008 ? might be 1.0184x slower
fold-put-by-id-to-multi-put-by-offset 11.4315+-0.8821 ? 11.7145+-0.8120 ? might be 1.0248x slower
fold-put-structure 8.6942+-1.1696 ? 8.8929+-1.0738 ? might be 1.0229x slower
for-of-iterate-array-entries 11.0456+-0.4550 ? 11.1745+-0.4394 ? might be 1.0117x slower
for-of-iterate-array-keys 5.1106+-0.1261 ? 5.3860+-0.3469 ? might be 1.0539x slower
for-of-iterate-array-values 4.3795+-0.3280 ? 4.7305+-0.2394 ? might be 1.0801x slower
fround 26.1556+-1.0184 25.7432+-0.0276 might be 1.0160x faster
ftl-library-inlining-dataview 159.5065+-20.3904 ? 166.5156+-27.6958 ? might be 1.0439x slower
ftl-library-inlining 134.2111+-3.2031 133.3567+-0.3098
function-dot-apply 2.8790+-0.1053 2.7940+-0.2984 might be 1.0304x faster
function-test 5.6203+-0.2980 5.5601+-0.1176 might be 1.0108x faster
function-with-eval 226.4252+-1.1450 ? 226.6873+-4.2426 ?
gcse-poly-get-less-obvious 36.7097+-0.9344 ? 37.1866+-0.7438 ? might be 1.0130x slower
gcse-poly-get 36.6384+-0.2324 ? 36.9536+-0.2619 ?
gcse 9.1887+-0.1535 9.1263+-0.1787
get-by-id-bimorphic-check-structure-elimination-simple
4.1465+-0.1181 4.0878+-0.0447 might be 1.0144x faster
get-by-id-bimorphic-check-structure-elimination
12.1324+-0.1044 12.0778+-0.1025
get-by-id-chain-from-try-block 19.7584+-0.1127 19.6074+-0.0805
get-by-id-check-structure-elimination 10.7896+-0.1714 10.7125+-0.1162
get-by-id-proto-or-self 33.2939+-4.5872 30.6615+-0.5304 might be 1.0859x faster
get-by-id-quadmorphic-check-structure-elimination-simple
5.1968+-0.1885 ? 5.3729+-0.3389 ? might be 1.0339x slower
get-by-id-self-or-proto 32.3140+-3.8927 ? 32.4664+-4.1033 ?
get-by-val-out-of-bounds 7.7048+-0.0937 ? 7.8212+-0.1789 ? might be 1.0151x slower
get_callee_monomorphic 6.4418+-0.5322 ? 6.5833+-0.9700 ? might be 1.0220x slower
get_callee_polymorphic 5.3914+-0.1723 ? 5.5063+-0.0768 ? might be 1.0213x slower
getter-no-activation 6.3140+-0.1034 ? 6.3192+-0.1278 ?
getter-richards 180.3203+-3.7006 176.7853+-7.2642 might be 1.0200x faster
getter 9.2660+-0.4927 9.2566+-0.4562
global-var-const-infer-fire-from-opt 1.6943+-0.0592 ? 1.8022+-0.0847 ? might be 1.0637x slower
global-var-const-infer 1.7415+-0.1462 1.6291+-0.1500 might be 1.0690x faster
HashMap-put-get-iterate-keys 51.6534+-1.1998 ? 51.7634+-1.0618 ?
HashMap-put-get-iterate 50.3761+-0.6012 50.0996+-0.8934
HashMap-string-put-get-iterate 48.1560+-0.6403 47.3760+-0.2342 might be 1.0165x faster
hoist-make-rope 17.6647+-2.6264 ? 19.1873+-3.1359 ? might be 1.0862x slower
hoist-poly-check-structure-effectful-loop
9.5908+-0.1093 ? 9.6189+-0.0365 ?
hoist-poly-check-structure 6.3116+-0.1337 ? 6.3143+-0.1068 ?
imul-double-only 13.8478+-2.1592 12.1512+-0.3126 might be 1.1396x faster
imul-int-only 14.3560+-0.7743 ? 14.5774+-0.0720 ? might be 1.0154x slower
imul-mixed 12.2758+-0.3281 ? 12.4580+-0.1690 ? might be 1.0148x slower
in-four-cases 35.3623+-0.2299 ? 35.5377+-0.3855 ?
in-one-case-false 18.7362+-0.1230 ? 18.9167+-0.0904 ?
in-one-case-true 18.7361+-0.2161 ? 18.7432+-0.1037 ?
in-two-cases 19.2914+-0.2106 19.2202+-0.0497
indexed-properties-in-objects 4.7818+-0.0994 4.7349+-0.1446
infer-closure-const-then-mov-no-inline 6.0425+-0.0580 5.9664+-0.0912 might be 1.0128x faster
infer-closure-const-then-mov 26.1909+-0.2291 26.0888+-0.2013
infer-closure-const-then-put-to-scope-no-inline
19.9008+-0.1173 19.8727+-0.1765
infer-closure-const-then-put-to-scope 33.9648+-0.9965 ? 34.1788+-0.7592 ?
infer-closure-const-then-reenter-no-inline
93.0925+-0.2930 92.6370+-0.8677
infer-closure-const-then-reenter 31.8092+-0.5085 ? 31.8775+-0.2082 ?
infer-constant-global-property 38.8815+-0.0816 ? 38.9991+-0.1381 ?
infer-constant-property 3.7441+-0.1425 3.6388+-0.2008 might be 1.0289x faster
infer-one-time-closure-ten-vars 17.6861+-0.3930 17.6125+-0.1956
infer-one-time-closure-two-vars 16.9491+-0.1937 ? 16.9615+-0.5703 ?
infer-one-time-closure 16.8242+-0.4588 16.7938+-0.5082
infer-one-time-deep-closure 29.8387+-0.2992 29.5775+-0.5055
inline-arguments-access 2.8828+-0.0841 2.8301+-0.1709 might be 1.0186x faster
inline-arguments-aliased-access 3.0391+-0.1706 ? 3.2081+-0.0639 ? might be 1.0556x slower
inline-arguments-local-escape 26.2448+-0.2466 ? 26.6507+-1.6650 ? might be 1.0155x slower
inline-get-scoped-var 5.9337+-0.2037 5.9230+-0.1995
inlined-put-by-id-transition 18.6192+-0.2993 ? 18.6315+-0.2925 ?
int-or-other-abs-then-get-by-val 9.5482+-0.1762 9.4762+-0.1543
int-or-other-abs-zero-then-get-by-val 34.4332+-0.5905 ? 34.4520+-0.8143 ?
int-or-other-add-then-get-by-val 8.4808+-0.1908 8.4376+-0.2232
int-or-other-add 9.0556+-0.2322 ? 9.1369+-0.3095 ?
int-or-other-div-then-get-by-val 6.5219+-0.1306 6.5076+-0.1235
int-or-other-max-then-get-by-val 8.0834+-0.2801 ? 8.2429+-0.3038 ? might be 1.0197x slower
int-or-other-min-then-get-by-val 7.2509+-0.1862 7.2227+-0.2425
int-or-other-mod-then-get-by-val 6.1520+-0.0339 6.1027+-0.1393
int-or-other-mul-then-get-by-val 6.5627+-0.0828 6.5493+-0.1748
int-or-other-neg-then-get-by-val 8.1566+-0.2424 8.1215+-0.2293
int-or-other-neg-zero-then-get-by-val 34.5405+-0.2778 34.1718+-0.4726 might be 1.0108x faster
int-or-other-sub-then-get-by-val 8.8137+-0.0916 8.7767+-0.2080
int-or-other-sub 6.8842+-0.1051 6.7810+-0.7515 might be 1.0152x faster
int-overflow-local 7.9830+-0.1900 7.9332+-0.1485
Int16Array-alloc-long-lived 77.7075+-0.9676 76.7988+-0.7093 might be 1.0118x faster
Int16Array-bubble-sort-with-byteLength 51.2611+-0.4691 51.1688+-0.2292
Int16Array-bubble-sort 49.8958+-0.3509 49.7271+-0.3523
Int16Array-load-int-mul 2.4927+-0.3321 2.3790+-0.1765 might be 1.0478x faster
Int16Array-to-Int32Array-set 117.2755+-5.7481 116.0438+-8.2719 might be 1.0106x faster
Int32Array-alloc-large 37.8079+-0.7289 37.5195+-0.3693
Int32Array-alloc-long-lived 84.8037+-0.6696 84.3643+-0.5691
Int32Array-alloc 4.9630+-0.1617 ? 5.0410+-0.1938 ? might be 1.0157x slower
Int32Array-Int8Array-view-alloc 13.2523+-1.1704 13.0984+-0.5665 might be 1.0117x faster
int52-spill 12.6750+-0.3402 ? 12.7317+-0.2315 ?
Int8Array-alloc-long-lived 71.4784+-0.8167 71.2983+-0.1291
Int8Array-load-with-byteLength 5.1890+-0.1435 5.1741+-0.1626
Int8Array-load 5.2203+-0.1241 5.1931+-0.1744
integer-divide 19.4474+-0.1624 19.2855+-0.0699
integer-modulo 3.7591+-0.3421 ? 3.7742+-0.1617 ?
large-int-captured 12.5248+-0.3290 ? 12.7825+-0.5772 ? might be 1.0206x slower
large-int-neg 30.6542+-0.4530 30.5390+-0.3885
large-int 27.8512+-0.2662 27.7445+-0.1375
logical-not 8.0995+-0.1499 ? 8.3277+-0.5641 ? might be 1.0282x slower
lots-of-fields 19.5097+-0.1337 19.4719+-0.3008
make-indexed-storage 5.3522+-0.2227 4.8837+-0.7897 might be 1.0959x faster
make-rope-cse 6.3777+-0.2408 ? 6.5562+-0.4396 ? might be 1.0280x slower
marsaglia-larger-ints 69.2272+-1.1604 68.7192+-1.3240
marsaglia-osr-entry 34.6951+-0.5846 34.5122+-0.3559
max-boolean 3.9263+-0.0762 3.9242+-0.1351
method-on-number 34.1494+-0.7148 ? 35.0854+-3.5781 ? might be 1.0274x slower
min-boolean 3.8738+-0.1329 3.8368+-0.1615
minus-boolean-double 4.3873+-0.1232 4.3200+-0.0727 might be 1.0156x faster
minus-boolean 3.7852+-0.0720 3.7033+-0.1755 might be 1.0221x faster
misc-strict-eq 71.0651+-1.1205 ? 76.1788+-9.0995 ? might be 1.0720x slower
mod-boolean-double 14.3724+-0.1671 ? 14.5200+-0.8415 ? might be 1.0103x slower
mod-boolean 9.2263+-0.1184 9.2256+-0.1656
mul-boolean-double 4.9919+-0.1161 ? 5.0269+-0.1229 ?
mul-boolean 4.1248+-0.0623 4.0256+-0.1719 might be 1.0246x faster
neg-boolean 4.5883+-0.0942 ? 4.6048+-0.0437 ?
negative-zero-divide 0.6188+-0.1386 ? 0.6452+-0.0898 ? might be 1.0426x slower
negative-zero-modulo 0.6545+-0.0473 0.6349+-0.1155 might be 1.0308x faster
negative-zero-negate 0.5855+-0.0879 0.5372+-0.0249 might be 1.0899x faster
nested-function-parsing 41.0352+-0.2690 40.8641+-0.2172
new-array-buffer-dead 4.1129+-0.2076 ? 4.1379+-0.3367 ?
new-array-buffer-push 10.7457+-0.2452 10.5203+-0.1413 might be 1.0214x faster
new-array-dead 17.6908+-0.9714 17.5629+-0.9695
new-array-push 8.3011+-0.1249 ? 8.3193+-0.2714 ?
number-test 4.9020+-0.4426 4.8317+-0.2495 might be 1.0145x faster
object-closure-call 10.5706+-0.1870 ? 10.7538+-0.0905 ? might be 1.0173x slower
object-test 5.3610+-0.2113 ? 5.3730+-0.1623 ?
obvious-sink-pathology-taken 217.3514+-1.9245 ^ 214.0780+-0.3054 ^ definitely 1.0153x faster
obvious-sink-pathology 201.2391+-2.6274 200.7522+-3.5779
obviously-elidable-new-object 55.7850+-0.9496 55.0394+-0.7010 might be 1.0135x faster
plus-boolean-arith 4.0974+-0.1436 4.0213+-0.0837 might be 1.0189x faster
plus-boolean-double 4.4755+-0.0946 4.4678+-0.0669
plus-boolean 3.5936+-0.1286 ? 3.6833+-0.2692 ? might be 1.0250x slower
poly-chain-access-different-prototypes-simple
5.1332+-0.0970 5.1332+-0.0854
poly-chain-access-different-prototypes 3.7897+-0.7005 3.5861+-0.2065 might be 1.0568x faster
poly-chain-access-simpler 5.1768+-0.1205 ? 5.1812+-0.1303 ?
poly-chain-access 3.5493+-0.1897 3.5275+-0.1414
poly-stricteq 101.1295+-2.5868 100.9315+-1.1071
polymorphic-array-call 3.3520+-0.2910 3.2274+-0.1553 might be 1.0386x faster
polymorphic-get-by-id 5.2880+-0.1104 5.2699+-0.1166
polymorphic-put-by-id 46.4290+-2.5495 ? 46.7411+-2.5766 ?
polymorphic-structure 32.9877+-0.1825 32.7559+-0.3972
polyvariant-monomorphic-get-by-id 17.1022+-1.0412 16.8517+-0.2065 might be 1.0149x faster
proto-getter-access 16.7443+-0.4297 16.5272+-0.3409 might be 1.0131x faster
put-by-id-replace-and-transition 13.8135+-0.1142 ^ 13.4194+-0.0740 ^ definitely 1.0294x faster
put-by-id-slightly-polymorphic 4.2306+-0.2011 4.0024+-0.1129 might be 1.0570x faster
put-by-id 20.6379+-0.6266 20.5039+-0.3636
put-by-val-direct 1.0159+-0.1372 0.9672+-0.0237 might be 1.0504x faster
put-by-val-large-index-blank-indexing-type
10.8410+-0.4722 ? 11.1888+-1.0078 ? might be 1.0321x slower
put-by-val-machine-int 4.3951+-0.2577 4.2426+-0.2174 might be 1.0359x faster
rare-osr-exit-on-local 25.5859+-0.2137 25.5680+-0.0928
register-pressure-from-osr 38.1308+-0.3462 ? 38.8505+-1.8452 ? might be 1.0189x slower
setter 7.2527+-0.0726 7.2230+-0.1075
simple-activation-demo 45.2758+-0.9283 44.9360+-0.4936
simple-getter-access 22.1892+-0.5301 22.1487+-0.3947
simple-poly-call-nested 13.9409+-0.7088 13.7840+-0.1337 might be 1.0114x faster
simple-poly-call 2.0372+-0.1697 ? 2.0515+-0.0422 ?
sin-boolean 29.6371+-0.4665 29.5812+-0.5918
sinkable-new-object-dag 117.4052+-0.9555 116.5355+-0.4832
sinkable-new-object-taken 93.4968+-5.9617 88.9117+-0.6971 might be 1.0516x faster
sinkable-new-object 61.2180+-0.8399 ? 61.3400+-1.4976 ?
slow-array-profile-convergence 5.1018+-0.2715 ? 5.3026+-0.1663 ? might be 1.0394x slower
slow-convergence 5.7303+-0.3998 5.6806+-0.4248
sparse-conditional 1.8288+-0.1456 ? 1.8464+-0.2429 ?
splice-to-remove 41.6455+-0.8918 ^ 35.4322+-2.6289 ^ definitely 1.1754x faster
string-char-code-at 30.4093+-0.3345 30.2682+-0.3253
string-concat-object 3.3947+-0.2665 3.3003+-0.2673 might be 1.0286x faster
string-concat-pair-object 3.4307+-0.2015 ? 3.4792+-0.4550 ? might be 1.0141x slower
string-concat-pair-simple 19.2665+-0.5413 18.7865+-0.5862 might be 1.0256x faster
string-concat-simple 19.9454+-0.3921 19.6539+-0.6129 might be 1.0148x faster
string-cons-repeat 12.4667+-0.4590 12.1314+-0.3106 might be 1.0276x faster
string-cons-tower 12.0494+-0.2723 11.5101+-0.3408 might be 1.0469x faster
string-equality 34.7372+-0.6414 ? 35.0696+-1.7369 ?
string-get-by-val-big-char 13.5543+-0.1765 13.3838+-0.1537 might be 1.0127x faster
string-get-by-val-out-of-bounds-insane 8.4212+-0.1948 ? 8.8743+-1.9815 ? might be 1.0538x slower
string-get-by-val-out-of-bounds 10.1138+-0.6395 9.8272+-0.1071 might be 1.0292x faster
string-get-by-val 6.8783+-0.0854 6.7597+-0.1664 might be 1.0175x faster
string-hash 3.5449+-0.0487 ? 3.6699+-0.1223 ? might be 1.0352x slower
string-long-ident-equality 27.2795+-1.1739 27.0955+-0.6206
string-repeat-arith 62.1400+-1.3519 61.9624+-0.5083
string-sub 120.3463+-0.8595 ? 120.8298+-0.3453 ?
string-test 4.8938+-0.2348 4.7329+-0.1307 might be 1.0340x faster
string-var-equality 62.1268+-0.3333 62.0968+-0.2072
structure-hoist-over-transitions 4.1060+-0.2878 ? 4.1290+-0.2895 ?
substring-concat-weird 74.5724+-0.8467 73.9078+-0.6376
substring-concat 78.8601+-0.6116 78.1986+-0.4697
substring 90.0316+-0.3427 89.7267+-0.3882
switch-char-constant 3.8243+-0.2032 3.8170+-0.1772
switch-char 12.2530+-1.0250 12.0240+-0.1224 might be 1.0190x faster
switch-constant 19.1667+-1.1769 18.3155+-0.1905 might be 1.0465x faster
switch-string-basic-big-var 27.8137+-1.4937 27.0532+-0.2000 might be 1.0281x faster
switch-string-basic-big 23.8311+-0.3789 23.6327+-0.3626
switch-string-basic-var 32.9052+-1.6893 32.4160+-1.4665 might be 1.0151x faster
switch-string-basic 26.6434+-2.2716 ? 27.9477+-1.5121 ? might be 1.0490x slower
switch-string-big-length-tower-var 31.2447+-0.1660 ? 31.3659+-0.3409 ?
switch-string-length-tower-var 26.1968+-0.2505 26.1537+-0.7635
switch-string-length-tower 20.9168+-0.2720 ? 21.0845+-0.3291 ?
switch-string-short 20.3360+-0.4472 20.2965+-0.1721
switch 27.1509+-0.8038 23.6587+-6.6594 might be 1.1476x faster
tear-off-arguments-simple 3.0574+-0.2098 3.0242+-0.2302 might be 1.0110x faster
tear-off-arguments 4.6716+-0.1961 4.5760+-0.1241 might be 1.0209x faster
temporal-structure 20.7015+-0.4431 20.6696+-0.1768
to-int32-boolean 24.9670+-0.2490 ? 25.0707+-0.1293 ?
undefined-property-access 582.8043+-1.7300 ? 584.1109+-5.1334 ?
undefined-test 4.9273+-0.0649 ? 5.0392+-0.0795 ? might be 1.0227x slower
unprofiled-licm 34.8692+-0.5498 34.5882+-0.1922
weird-inlining-const-prop 3.3654+-0.2688 ? 3.4314+-0.1782 ? might be 1.0196x slower
<arithmetic> 30.7965+-0.0675 30.6547+-0.1106 might be 1.0046x faster
<geometric> * 13.7433+-0.0428 13.6871+-0.0692 might be 1.0041x faster
<harmonic> 6.8824+-0.1087 6.8480+-0.0891 might be 1.0050x faster
Baseline GCDontDropLocks
AsmBench:
bigfib.cpp 892.8593+-11.0205 884.5098+-12.9579
cray.c 937.8826+-9.8051 934.5866+-7.5146
dry.c 1054.9327+-2.1557 1038.5106+-50.3661 might be 1.0158x faster
FloatMM.c 1302.4790+-2.9021 ? 1304.0134+-5.5945 ?
gcc-loops.cpp 8249.8732+-8.1224 8245.3165+-46.8821
n-body.c 2459.3489+-1.5754 ? 2463.0585+-4.0248 ?
Quicksort.c 711.7990+-12.7685 ? 715.6802+-2.4469 ?
stepanov_container.cpp 6171.9437+-126.7963 ? 6200.2064+-103.5813 ?
Towers.c 604.5075+-1.6238 ? 605.1746+-1.3776 ?
<arithmetic> 2487.2918+-15.4417 ? 2487.8952+-12.5238 ? might be 1.0002x slower
<geometric> * 1581.7820+-6.6192 1579.0328+-7.2450 might be 1.0017x faster
<harmonic> 1176.2988+-4.4010 1173.5011+-5.4085 might be 1.0024x faster
Baseline GCDontDropLocks
CompressionBench:
huffman 1053.0552+-7.4523 1052.2343+-7.6608
arithmetic-simple 767.8580+-6.4668 ? 771.0495+-4.0561 ?
arithmetic-precise 594.5696+-4.5895 ? 594.6685+-4.6868 ?
arithmetic-complex-precise 597.4836+-8.7574 596.3629+-9.3928
arithmetic-precise-order-0 888.8469+-12.1076 ? 892.4737+-3.0690 ?
arithmetic-precise-order-1 649.8916+-6.8364 ? 655.4582+-23.2516 ?
arithmetic-precise-order-2 737.1080+-39.4436 701.8575+-11.0504 might be 1.0502x faster
arithmetic-simple-order-1 799.6346+-29.9775 793.8840+-17.2383
arithmetic-simple-order-2 875.8927+-5.6488 872.1931+-15.2694
lz-string 633.1583+-30.5944 631.7170+-28.3004
<arithmetic> 759.7499+-7.8170 756.1899+-2.6853 might be 1.0047x faster
<geometric> * 747.1227+-8.4012 743.5275+-3.0568 might be 1.0048x faster
<harmonic> 735.1799+-8.8974 731.6467+-3.4965 might be 1.0048x faster
Baseline GCDontDropLocks
All benchmarks:
<arithmetic> 188.5052+-0.1356 188.4297+-0.1818 might be 1.0004x faster
<geometric> 23.4283+-0.0664 23.3506+-0.0895 might be 1.0033x faster
<harmonic> 5.9626+-0.0663 5.9286+-0.0509 might be 1.0057x faster
Baseline GCDontDropLocks
Geomean of preferred means:
<scaled-result> 120.8835+-0.4227 120.6260+-0.2997 might be 1.0021x faster
Comment on attachment 246072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246072&action=review I'm still not sure about the effects on peak memory usage for ObjC apps that uses long running JS code which also allocates a lot of JS wrapped ObjC objects. However, it is re-assuring that release of JS objects isn't gated by this change. I agree that we can go with this solution until there is evidence to the contrary that the peak memory usage is of consequence. I have added a few more questions and comments. See below. >>> Source/JavaScriptCore/ChangeLog:7 >>> + >> >> Your ChangeLog comment describes what was done but not why. I think your description of the issue in bugzilla did a good job of explaining what the issue is. Would you mind adding it here before you start explaining how it is fixed? > > I added this paragraph to the top of the ChangeLog entry: > The root issue is that one thread, while allocating JS objects may need to garbage collect. > During that garbage collection, if there are delayed Objective C objects that need to be > released, we drop the locks to release them. While the locks are release by that thread, > another thread can enter the VM and might have exactly the same source and therefore hit > this issue. This fixes the problem by not dropping the locks during garbage collection. I feel that the detail about the code cache having a partially initialized entry is important to understanding this issue. How about ... The root issue is that one thread, while parsing source code into an UnlinkedCodeBlock, may need to garbage collect. During that garbage collection, if there are delayed Objective C objects that need to be released, we drop the locks to release them. While the locks are release by that thread, another thread can enter the VM and might parse exactly the same source and therefore hit this issue where the CodeCache contains an incomplete entry for that source from the first thread. This fixes the problem by not dropping the locks during garbage collection. >>> Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:1758 >>> +</Project> >> >> Is this necessary and ok to do? > > Given that it is XML, I think it is fine. The Win EWS bot built with the patch just fine. Would you mind reverting this line, or at least deleting the "\No newline at end of file"? This change is just not relevant to this patch, and having the "\No newline at end of file" before the end of the file just feels icky. >>> Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:4390 >>> +</Project> >> >> Ditto. > > Fixed. Ditto if your fixed does not mean reverting this change, or deleting the "\No newline at end of file". > Source/JavaScriptCore/heap/Heap.cpp:376 > + if (!m_delayedReleaseRecursionCount++) { > + while (!m_delayedReleaseObjects.isEmpty()) { > + RetainPtr<CFTypeRef> objectToRelease = m_delayedReleaseObjects.takeLast(); > + objectToRelease.clear(); > + } > + } In the original ~DelayedReleaseScope(), we do the release by simply calling m_delayedReleaseObjects.clear(). Why do we need to iterate it here and manually clear each entry? Another question: why do we need this m_delayedReleaseRecursionCount? Given that releaseDelayedReleasedObjects() is only called just before the outermost JSLock is unlocked, and on heap destruction, there should be no chance of m_delayedReleaseRecursionCount ever being more than 0. Am I missing something? > Source/JavaScriptCore/heap/MarkedAllocator.cpp:132 > - // Due to the DelayedReleaseScope in tryAllocateHelper, some other thread might have > + // Due to the DelayedReleaseList in tryAllocateHelper, some other thread might have > // created a new block after we thought we didn't find any free cells. > while (!result && m_currentBlock) { > // A new block was added by another thread so try popping the free list. I think this while loop is now obsolete, and this comment is invalid. Since we no longer have a DelayedReleaseScope in tryAllocateHelper(), there's now no opportunity for another thread to run and create the new block that we're checking for here. Hence, this loop and comment should be removed. (In reply to comment #5) > Comment on attachment 246072 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=246072&action=review > > I'm still not sure about the effects on peak memory usage for ObjC apps that > uses long running JS code which also allocates a lot of JS wrapped ObjC > objects. However, it is re-assuring that release of JS objects isn't gated > by this change. I agree that we can go with this solution until there is > evidence to the contrary that the peak memory usage is of consequence. > > I have added a few more questions and comments. See below. > > >>> Source/JavaScriptCore/ChangeLog:7 > >>> + > >> > >> Your ChangeLog comment describes what was done but not why. I think your description of the issue in bugzilla did a good job of explaining what the issue is. Would you mind adding it here before you start explaining how it is fixed? > > > > I added this paragraph to the top of the ChangeLog entry: > > The root issue is that one thread, while allocating JS objects may need to garbage collect. > > During that garbage collection, if there are delayed Objective C objects that need to be > > released, we drop the locks to release them. While the locks are release by that thread, > > another thread can enter the VM and might have exactly the same source and therefore hit > > this issue. This fixes the problem by not dropping the locks during garbage collection. > > I feel that the detail about the code cache having a partially initialized > entry is important to understanding this issue. How about ... > > The root issue is that one thread, while parsing source code into an > UnlinkedCodeBlock, may need to garbage collect. During that garbage > collection, if there are delayed Objective C objects that need to be > released, we drop the locks to release them. While the locks are release by > that thread, another thread can enter the VM and might parse exactly the > same source and therefore hit this issue where the CodeCache contains an > incomplete entry for that source from the first thread. This fixes the > problem by not dropping the locks during garbage collection. During GC, we used to drop all lock even if there aren't any delayed Objective C objects that need to be released. How about: The issue for this bug is that one thread, takes a cache miss in CodeCache::getGlobalCodeBlock, but in the process creates a cache entry with a nullptr UnlinkedCodeBlockType* which it will fill in later in the function. During the body of that function, it allocates objects that may garbage collect. During that garbage collection, we drop the all locks. While the locks are released by the first thread, another thread can enter the VM and might have exactly the same source and enter CodeCache::getGlobalCodeBlock() itself. When it looks up the code block, it sees it as a cache it and uses the nullptr UnlinkedCodeBlockType* and crashes. This fixes the problem by not dropping the locks during garbage collection. There are other likely scenarios where we have a date structure like this code cache in an unsafe state for arbitrary reentrance. > >>> Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj:1758 > >>> +</Project> > >> > >> Is this necessary and ok to do? > > > > Given that it is XML, I think it is fine. The Win EWS bot built with the patch just fine. > > Would you mind reverting this line, or at least deleting the "\No newline at > end of file"? This change is just not relevant to this patch, and having > the "\No newline at end of file" before the end of the file just feels icky. Fine. > >>> Source/JavaScriptCore/JavaScriptCore.vcxproj/JavaScriptCore.vcxproj.filters:4390 > >>> +</Project> > >> > >> Ditto. > > > > Fixed. > > Ditto if your fixed does not mean reverting this change, or deleting the > "\No newline at end of file". I'll revert. > > Source/JavaScriptCore/heap/Heap.cpp:376 > > + if (!m_delayedReleaseRecursionCount++) { > > + while (!m_delayedReleaseObjects.isEmpty()) { > > + RetainPtr<CFTypeRef> objectToRelease = m_delayedReleaseObjects.takeLast(); > > + objectToRelease.clear(); > > + } > > + } > > In the original ~DelayedReleaseScope(), we do the release by simply calling > m_delayedReleaseObjects.clear(). Why do we need to iterate it here and > manually clear each entry? > > Another question: why do we need this m_delayedReleaseRecursionCount? Given > that releaseDelayedReleasedObjects() is only called just before the > outermost JSLock is unlocked, and on heap destruction, there should be no > chance of m_delayedReleaseRecursionCount ever being more than 0. Am I > missing something? Yes. As we clear all the delayed release objects, the release of that object could invoke some arbitrary JS that creates more delayed objects to be released. This means that we can't clear the list with Vector::clear() and we will be in the middle of the clear, but recursively we'll be appending and then clearing as that recursive call executes the same path. That is why we also have a recursive count so that we are only releasing at the first call and not any nested calls. testapi.mm hits this case, see EvilAllocationObject. That's why this section is in the ChangeLog: We do need to guard against the case that releasing an object can create more objects by calling into JS. This case is acually tested by testapi.mm. I'll add more to it. > > Source/JavaScriptCore/heap/MarkedAllocator.cpp:132 > > - // Due to the DelayedReleaseScope in tryAllocateHelper, some other thread might have > > + // Due to the DelayedReleaseList in tryAllocateHelper, some other thread might have > > // created a new block after we thought we didn't find any free cells. > > while (!result && m_currentBlock) { > > // A new block was added by another thread so try popping the free list. > > I think this while loop is now obsolete, and this comment is invalid. Since > we no longer have a DelayedReleaseScope in tryAllocateHelper(), there's now > no opportunity for another thread to run and create the new block that we're > checking for here. Hence, this loop and comment should be removed. I think these can be removed. I'll remove these and rerun the tests. Comment on attachment 246072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246072&action=review r=me with remaining changes and tests passing. >>>>> Source/JavaScriptCore/ChangeLog:7 >>>>> + >>>> >>>> Your ChangeLog comment describes what was done but not why. I think your description of the issue in bugzilla did a good job of explaining what the issue is. Would you mind adding it here before you start explaining how it is fixed? >>> >>> I added this paragraph to the top of the ChangeLog entry: >>> The root issue is that one thread, while allocating JS objects may need to garbage collect. >>> During that garbage collection, if there are delayed Objective C objects that need to be >>> released, we drop the locks to release them. While the locks are release by that thread, >>> another thread can enter the VM and might have exactly the same source and therefore hit >>> this issue. This fixes the problem by not dropping the locks during garbage collection. >> >> I feel that the detail about the code cache having a partially initialized entry is important to understanding this issue. How about ... >> >> The root issue is that one thread, while parsing source code into an UnlinkedCodeBlock, may need to garbage collect. During that garbage collection, if there are delayed Objective C objects that need to be released, we drop the locks to release them. While the locks are release by that thread, another thread can enter the VM and might parse exactly the same source and therefore hit this issue where the CodeCache contains an incomplete entry for that source from the first thread. This fixes the problem by not dropping the locks during garbage collection. > > During GC, we used to drop all lock even if there aren't any delayed Objective C objects that need to be released. > > How about: > > The issue for this bug is that one thread, takes a cache miss in CodeCache::getGlobalCodeBlock, > but in the process creates a cache entry with a nullptr UnlinkedCodeBlockType* which it will > fill in later in the function. During the body of that function, it allocates objects that > may garbage collect. During that garbage collection, we drop the all locks. While the > locks are released by the first thread, another thread can enter the VM and might have exactly > the same source and enter CodeCache::getGlobalCodeBlock() itself. When it looks up the code block, > it sees it as a cache it and uses the nullptr UnlinkedCodeBlockType* and crashes. This fixes > the problem by not dropping the locks during garbage collection. There are other likely scenarios > where we have a date structure like this code cache in an unsafe state for arbitrary reentrance. Looks good. typo: "date" ==> "data". >>> Source/JavaScriptCore/heap/Heap.cpp:376 >>> + } >> >> In the original ~DelayedReleaseScope(), we do the release by simply calling m_delayedReleaseObjects.clear(). Why do we need to iterate it here and manually clear each entry? >> >> Another question: why do we need this m_delayedReleaseRecursionCount? Given that releaseDelayedReleasedObjects() is only called just before the outermost JSLock is unlocked, and on heap destruction, there should be no chance of m_delayedReleaseRecursionCount ever being more than 0. Am I missing something? > > Yes. As we clear all the delayed release objects, the release of that object could invoke some arbitrary JS that creates more delayed objects to be released. This means that we can't clear the list with Vector::clear() and we will be in the middle of the clear, but recursively we'll be appending and then clearing as that recursive call executes the same path. That is why we also have a recursive count so that we are only releasing at the first call and not any nested calls. testapi.mm hits this case, see EvilAllocationObject. > > That's why this section is in the ChangeLog: > We do need to guard against the case that releasing an object can create more objects > by calling into JS. This case is acually tested by testapi.mm. > > I'll add more to it. I see. That's clear now. Can you put that comment in this block of code since it is not automatically clear from just reading the code as is? Committed r179728: <http://trac.webkit.org/changeset/179728> Comment on attachment 246072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=246072&action=review > Source/JavaScriptCore/heap/Heap.cpp:374 > + RetainPtr<CFTypeRef> objectToRelease = m_delayedReleaseObjects.takeLast(); > + objectToRelease.clear(); I agree with Mark that you should have added a comment here. Please do that. Also, there's no need for an explicit temporary: You can just takeLast(). > Source/JavaScriptCore/runtime/JSLock.cpp:175 > + m_vm->heap.releaseDelayedReleasedObjects(); This looks wrong. If you release ObjC objects *before* releasing the lock, then all dealloc methods will run with the lock still held, and may deadlock with other running threads. The main goal of the code you removed was to avoid that kind of deadlock. So, I think you've introduced a regression where a dealloc method can now deadlock if it waits on a lock owned by a thread waiting to run JavaScript. A general rule in our API is that we never call out to client code while holding the lock. In fact, releaseDelayedObjectsNow() should ASSERT that the lock is not held, like so: (1) ASSERT that lock is not held (2) Acquire lock (3) Increment recursion counter and do recursion check (4) Take all items into temporary vector (5) Release lock (6) If temporary vector is empty, return (7) Clear temporary vector (8) Goto (1) > (1) ASSERT that lock is not held
...ASSERT that lock is not held *by current thread* (other threads are OK)
Reopened to address ggaren's concerns. (In reply to comment #9) > A general rule in our API is that we never call out to client code while > holding the lock. In fact, releaseDelayedObjectsNow() should ASSERT that the > lock is not held, like so: > > (1) ASSERT that lock is not by current thread held > (2) Acquire lock > (3) Increment recursion counter and do recursion check > (4) Take all items into temporary vector > (5) Release lock > (6) If temporary vector is empty, return > (7) Clear temporary vector > (8) Goto (1) Tried implementing these steps and there is a problem with infinite recursion. The first time through, you proceed through steps 1-5. When the lock is released at step 5, we reenter performing steps 1, 2 and 3 at which point we realize that we have entered recursively. In the process of exiting, we drop the lock and reenter. I have implemented a variation where by we enter holding the lock. (1) Increment recursion counter and do recursion check and exit if recursing (2) ASSERT that lock is held by current thread (3) Take all items into temporary Vector (4) If temporary Vector is empty, return (5) Release lock (6) Clear temporary vector (7) Reaquire lock (8) Goto (2) Not only is this recursive safe, but it satisfies the requirement that we release objects while not holding the lock. I also added the requested comments. Created attachment 247495 [details]
Updated patch.
Comment on attachment 247495 [details] Updated patch. View in context: https://bugs.webkit.org/attachment.cgi?id=247495&action=review > Source/JavaScriptCore/heap/Heap.cpp:394 > + Vector<RetainPtr<CFTypeRef>> objectsToRelease; > + objectsToRelease.swap(m_delayedReleaseObjects); objectsToRelease = WTF::move(m_delayedReleaseObjects) > Source/JavaScriptCore/heap/Heap.cpp:404 > + while (!objectsToRelease.isEmpty()) > + objectsToRelease.takeLast().clear(); If this code is hot: takeLast() is costly, especially if you chain them. Here you could loop over the whole array, clear every retain ptr, then the whole array backbuffer would be cleared when getting out of scope. Created attachment 247499 [details]
Updated with Benjamin's suggestions.
Comment on attachment 247499 [details] Updated with Benjamin's suggestions. View in context: https://bugs.webkit.org/attachment.cgi?id=247499&action=review > Source/JavaScriptCore/heap/Heap.cpp:390 > + while (1) { Can this be "while (m_delayedReleaseObjects.size())"? > Source/JavaScriptCore/heap/Heap.cpp:394 > + Vector<RetainPtr<CFTypeRef>> objectsToRelease; > + objectsToRelease = WTF::move(m_delayedReleaseObjects); Can this be one line instead of two? Side questions: (1) Can you update the title of this bug to reflect what's actually going on? We don't expect the CodeCache to be thread-safe -- or any other data structure in JSC -- so it's confusing to have a but title about CodeCache not being thread-safe. (2) Can you turn your test case into a regression test? (In reply to comment #16) > Comment on attachment 247499 [details] > Updated with Benjamin's suggestions. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=247499&action=review > > > Source/JavaScriptCore/heap/Heap.cpp:390 > > + while (1) { > > Can this be "while (m_delayedReleaseObjects.size())"? > > > Source/JavaScriptCore/heap/Heap.cpp:394 > > + Vector<RetainPtr<CFTypeRef>> objectsToRelease; > > + objectsToRelease = WTF::move(m_delayedReleaseObjects); > > Can this be one line instead of two? Yes on both counts. I prefer "while (!m_delayedReleaseObjects.empty())". Done in my source tree. (In reply to comment #17) > Side questions: > > (1) Can you update the title of this bug to reflect what's actually going > on? We don't expect the CodeCache to be thread-safe -- or any other data > structure in JSC -- so it's confusing to have a but title about CodeCache > not being thread-safe. How about "DelayedReleaseScope drops locks during GC which can cause a thread switch." > (2) Can you turn your test case into a regression test? This is already tested in the testApi "EvilAllocationObject" test case. Created attachment 247820 [details]
Updated patch with regression test
The Objective-C++ anonymous block functions declarations fail the style checker.
Attachment 247820 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:133: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:144: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:157: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:167: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:175: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:236: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:246: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:298: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:339: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:359: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:367: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:371: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:375: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 13 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 247820 [details]
Updated patch with regression test
r=me
Created attachment 247824 [details]
Patch for landing with speculative fix for pre 10.10 Mac OS X builds
Attachment 247824 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:132: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:143: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:156: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:166: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:174: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:235: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:245: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:297: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:338: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:358: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:366: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:370: Place brace on its own line for function definitions. [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/API/tests/Regress141275.mm:374: Place brace on its own line for function definitions. [whitespace/braces] [4]
Total errors found: 13 in 7 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r180992: <http://trac.webkit.org/changeset/180992> |
In CodeCache::getGlobalCodeBlock(), the CodeCacheMap::add() method will initially add a null UnlinkedCodeBlockType* and then set the value to the new code block after it is created. The creation of that code block could block, drop all of its locks and allow another thread to begin executing. That second thread could also add the same source to the CadeCache and this time it finds the entry the first thread created which has a null UnlinkedCodeBlockType*. This second thread will try to use that null pointer and then crash. Here is a sample stack trace for the first thread: * thread #18: tid = 0x8756a0, 0x000000010479c076 libsystem_kernel.dylib`__psynch_mutexwait + 10, queue = 'JSTEval' * frame #0: 0x000000010479c076 libsystem_kernel.dylib`__psynch_mutexwait + 10 frame #1: 0x00000001048ca744 libsystem_pthread.dylib`_pthread_mutex_lock + 482 frame #2: 0x0000000104391f67 libc++.1.dylib`std::__1::mutex::lock() + 9 frame #3: 0x000000010069353d JavaScriptCore`JSC::JSLock::lock(this=0x0000000119ffb000, lockCount=2) + 157 at JSLock.cpp:121 frame #4: 0x0000000100693cea JavaScriptCore`JSC::JSLock::grabAllLocks(this=0x0000000119ffb000, dropper=0x000000011b7358c8, droppedLockCount=2) + 186 at JSLock.cpp:237 frame #5: 0x000000010069407d JavaScriptCore`JSC::JSLock::DropAllLocks::~DropAllLocks(this=0x000000011b7358c8) + 109 at JSLock.cpp:280 frame #6: 0x0000000100694105 JavaScriptCore`JSC::JSLock::DropAllLocks::~DropAllLocks(this=0x000000011b7358c8) + 21 at JSLock.cpp:277 frame #7: 0x000000010051ce84 JavaScriptCore`JSC::DelayedReleaseScope::~DelayedReleaseScope(this=0x000000011b7359e0) + 260 at DelayedReleaseScope.h:57 frame #8: 0x0000000100512615 JavaScriptCore`JSC::DelayedReleaseScope::~DelayedReleaseScope(this=0x000000011b7359e0) + 21 at DelayedReleaseScope.h:47 frame #9: 0x0000000100773de0 JavaScriptCore`JSC::MarkedAllocator::tryAllocateHelper(this=0x0000000118013e78, bytes=336) + 672 at MarkedAllocator.cpp:105 frame #10: 0x0000000100772212 JavaScriptCore`JSC::MarkedAllocator::tryAllocate(this=0x0000000118013e78, bytes=336) + 114 at MarkedAllocator.cpp:129 frame #11: 0x0000000100771afe JavaScriptCore`JSC::MarkedAllocator::allocateSlowCase(this=0x0000000118013e78, bytes=336) + 254 at MarkedAllocator.cpp:171 frame #12: 0x000000010003c8a1 JavaScriptCore`JSC::MarkedAllocator::allocate(this=0x0000000118013e78, bytes=336) + 81 at MarkedAllocator.h:95 frame #13: 0x000000010004da59 JavaScriptCore`JSC::MarkedSpace::allocateWithImmortalStructureDestructor(this=0x0000000118010328, bytes=336) + 41 at MarkedSpace.h:251 frame #14: 0x000000010004da26 JavaScriptCore`JSC::Heap::allocateWithImmortalStructureDestructor(this=0x0000000118010018, bytes=336) + 118 at HeapInlines.h:196 frame #15: 0x0000000100122ac7 JavaScriptCore`void* JSC::allocateCell<JSC::UnlinkedProgramCodeBlock>(heap=0x0000000118010018, size=336) + 151 at JSCellInlines.h:134 frame #16: 0x000000010012284c JavaScriptCore`void* JSC::allocateCell<JSC::UnlinkedProgramCodeBlock>(heap=0x0000000118010018) + 28 at JSCellInlines.h:150 frame #17: 0x00000001001225e3 JavaScriptCore`JSC::UnlinkedProgramCodeBlock::create(vm=0x0000000118010000, info=0x000000011b735d48) + 35 at UnlinkedCodeBlock.h:622 frame #18: 0x000000010011daa0 JavaScriptCore`JSC::UnlinkedProgramCodeBlock* JSC::CodeCache::getGlobalCodeBlock<JSC::UnlinkedProgramCodeBlock, JSC::ProgramExecutable>(this=0x0000000119ff7058, vm=0x0000000118010000, executable=0x000000011a33fc70, source=0x000000011a33fcb0, strictness=JSParseNormal, debuggerMode=DebuggerOff, profilerMode=ProfilerOff, error=0x000000011b736230) + 1856 at CodeCache.cpp:107 frame #19: 0x000000010011c9de JavaScriptCore`JSC::CodeCache::getProgramCodeBlock(this=0x0000000119ff7058, vm=0x0000000118010000, executable=0x000000011a33fc70, source=0x000000011a33fcb0, strictness=JSParseNormal, debuggerMode=DebuggerOff, profilerMode=ProfilerOff, error=0x000000011b736230) + 94 at CodeCache.cpp:128 frame #20: 0x000000010065da9b JavaScriptCore`JSC::JSGlobalObject::createProgramCodeBlock(this=0x000000011a2df970, callFrame=0x000000011a2df9b0, executable=0x000000011a33fc70, exception=0x000000011b736388) + 315 at JSGlobalObject.cpp:765 frame #21: 0x000000010043ad9d JavaScriptCore`JSC::ProgramExecutable::initializeGlobalProperties(this=0x000000011a33fc70, vm=0x0000000118010000, callFrame=0x000000011a2df9b0, scope=0x000000011a2df970) + 253 at Executable.cpp:489 frame #22: 0x00000001005c37f3 JavaScriptCore`JSC::Interpreter::execute(this=0x0000000119ff4000, program=0x000000011a33fc70, callFrame=0x000000011a2df9b0, thisObj=0x000000011a2efaf0) + 3795 at Interpreter.cpp:889 frame #23: 0x00000001001448e0 JavaScriptCore`JSC::evaluate(exec=0x000000011a2df9b0, source=0x000000011b737b10, thisValue=JSValue at 0x000000011b737a70, returnedException=0x000000011b737ae0) + 480 at Completion.cpp:81 frame #24: 0x000000010062cfea JavaScriptCore`JSEvaluateScript(ctx=0x000000011a2df9b0, script=0x0000000119fd03c0, thisObject=0x0000000000000000, sourceURL=0x0000000000000000, startingLineNumber=1, exception=0x000000011b737c00) + 538 at JSBase.cpp:66 frame #25: 0x000000010064562e JavaScriptCore`-[JSContext evaluateScript:withSourceURL:](self=0x0000628000043210, _cmd=0x0000000100c914fc, script=0x0000000100008760, sourceURL=0x0000000000000000) + 158 at JSContext.mm:102 frame #26: 0x0000000100645582 JavaScriptCore`-[JSContext evaluateScript:](self=0x0000628000043210, _cmd=0x0000000100006f27, script=0x0000000100008760) + 66 at JSContext.mm:94 frame #27: 0x0000000100002f17 JSCTester`__42-[JSTEvaluator evaluateScript:completion:]_block_invoke(.block_descriptor=0x0000628000041230, context=0x0000628000043210) + 71 at JSTEvaluator.m:172 frame #28: 0x0000000100004c0c JSCTester`__30-[JSTEvaluator _sourcePerform]_block_invoke158(.block_descriptor=<unavailable>) + 220 at JSTEvaluator.m:343 frame #29: 0x000000010459cc64 libdispatch.dylib`_dispatch_call_block_and_release + 12 frame #30: 0x00000001045976ec libdispatch.dylib`_dispatch_client_callout + 8 frame #31: 0x00000001045ae4cf libdispatch.dylib`_dispatch_async_redirect_invoke + 2746 frame #32: 0x00000001045976ec libdispatch.dylib`_dispatch_client_callout + 8 frame #33: 0x000000010459b9a4 libdispatch.dylib`_dispatch_root_queue_drain + 2858 frame #34: 0x000000010459ae73 libdispatch.dylib`_dispatch_worker_thread3 + 106 frame #35: 0x00000001048ccb1d libsystem_pthread.dylib`_pthread_wqthread + 729 frame #36: 0x00000001048ca489 libsystem_pthread.dylib`start_wqthread + 13 Here is the stack trace for the second thread: * thread #6: tid = 0x875633, 0x000000010011f43c JavaScriptCore`JSC::UnlinkedCodeBlock::firstLine(this=0x0000000000000000) const + 12 at UnlinkedCodeBlock.h:485, queue = 'JSTEval', stop reason = EXC_BAD_ACCESS (code=1, address=0x48) * frame #0: 0x000000010011f43c JavaScriptCore`JSC::UnlinkedCodeBlock::firstLine(this=0x0000000000000000) const + 12 at UnlinkedCodeBlock.h:485 frame #1: 0x000000010011d538 JavaScriptCore`JSC::UnlinkedProgramCodeBlock* JSC::CodeCache::getGlobalCodeBlock<JSC::UnlinkedProgramCodeBlock, JSC::ProgramExecutable>(this=0x0000000119ff7058, vm=0x0000000118010000, executable=0x000000011a33fb70, source=0x000000011a33fbb0, strictness=JSParseNormal, debuggerMode=DebuggerOff, profilerMode=ProfilerOff, error=0x000000010e102230) + 472 at CodeCache.cpp:85 frame #2: 0x000000010011c9de JavaScriptCore`JSC::CodeCache::getProgramCodeBlock(this=0x0000000119ff7058, vm=0x0000000118010000, executable=0x000000011a33fb70, source=0x000000011a33fbb0, strictness=JSParseNormal, debuggerMode=DebuggerOff, profilerMode=ProfilerOff, error=0x000000010e102230) + 94 at CodeCache.cpp:128 frame #3: 0x000000010065da9b JavaScriptCore`JSC::JSGlobalObject::createProgramCodeBlock(this=0x000000011a2df970, callFrame=0x000000011a2df9b0, executable=0x000000011a33fb70, exception=0x000000010e102388) + 315 at JSGlobalObject.cpp:765 frame #4: 0x000000010043ad9d JavaScriptCore`JSC::ProgramExecutable::initializeGlobalProperties(this=0x000000011a33fb70, vm=0x0000000118010000, callFrame=0x000000011a2df9b0, scope=0x000000011a2df970) + 253 at Executable.cpp:489 frame #5: 0x00000001005c37f3 JavaScriptCore`JSC::Interpreter::execute(this=0x0000000119ff4000, program=0x000000011a33fb70, callFrame=0x000000011a2df9b0, thisObj=0x000000011a2efaf0) + 3795 at Interpreter.cpp:889 frame #6: 0x00000001001448e0 JavaScriptCore`JSC::evaluate(exec=0x000000011a2df9b0, source=0x000000010e103b10, thisValue=JSValue at 0x000000010e103a70, returnedException=0x000000010e103ae0) + 480 at Completion.cpp:81 frame #7: 0x000000010062cfea JavaScriptCore`JSEvaluateScript(ctx=0x000000011a2df9b0, script=0x0000000119fe6060, thisObject=0x0000000000000000, sourceURL=0x0000000000000000, startingLineNumber=1, exception=0x000000010e103c00) + 538 at JSBase.cpp:66 frame #8: 0x000000010064562e JavaScriptCore`-[JSContext evaluateScript:withSourceURL:](self=0x0000628000043210, _cmd=0x0000000100c914fc, script=0x0000000100008760, sourceURL=0x0000000000000000) + 158 at JSContext.mm:102 frame #9: 0x0000000100645582 JavaScriptCore`-[JSContext evaluateScript:](self=0x0000628000043210, _cmd=0x0000000100006f27, script=0x0000000100008760) + 66 at JSContext.mm:94 frame #10: 0x0000000100002f17 JSCTester`__42-[JSTEvaluator evaluateScript:completion:]_block_invoke(.block_descriptor=0x0000628000041440, context=0x0000628000043210) + 71 at JSTEvaluator.m:172 frame #11: 0x0000000100004c0c JSCTester`__30-[JSTEvaluator _sourcePerform]_block_invoke158(.block_descriptor=<unavailable>) + 220 at JSTEvaluator.m:343 frame #12: 0x000000010459cc64 libdispatch.dylib`_dispatch_call_block_and_release + 12 frame #13: 0x00000001045976ec libdispatch.dylib`_dispatch_client_callout + 8 frame #14: 0x00000001045ae4cf libdispatch.dylib`_dispatch_async_redirect_invoke + 2746 frame #15: 0x00000001045976ec libdispatch.dylib`_dispatch_client_callout + 8 frame #16: 0x000000010459b9a4 libdispatch.dylib`_dispatch_root_queue_drain + 2858 frame #17: 0x000000010459ae73 libdispatch.dylib`_dispatch_worker_thread3 + 106 frame #18: 0x00000001048ccb1d libsystem_pthread.dylib`_pthread_wqthread + 729 frame #19: 0x00000001048ca489 libsystem_pthread.dylib`start_wqthread + 13 This was found with an internal multi-threaded test app. rdar://problem/19708290