WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
71977
DFG should not reparse code that was just parsed
https://bugs.webkit.org/show_bug.cgi?id=71977
Summary
DFG should not reparse code that was just parsed
Filip Pizlo
Reported
2011-11-09 17:41:23 PST
The DFG will currently redo parsing on an executable when that executable had just been parsed by the old JIT. It will also repeatedly reparse code that is inlined multiple times. This is likely not particularly great for performance. This is an umbrella bug for now, since there are a number of independent things that need to be fixed.
Attachments
work in progress
(49.28 KB, patch)
2011-11-10 00:23 PST
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
work in progress
(55.19 KB, patch)
2011-11-10 03:39 PST
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
the patch
(57.09 KB, patch)
2011-11-10 04:07 PST
,
Filip Pizlo
ggaren
: review+
ggaren
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2011-11-10 00:23:33 PST
Created
attachment 114448
[details]
work in progress Putting it up here as backup. I'm pretty sure this won't compile yet.
Filip Pizlo
Comment 2
2011-11-10 03:39:13 PST
Created
attachment 114465
[details]
work in progress It's starting to work.
Filip Pizlo
Comment 3
2011-11-10 03:54:17 PST
Not bad. Benchmark report for SunSpider, V8, and Kraken on oldmac.local (MacPro4,1). VMs tested: "TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (
r99833
) "DontReparse" at /Volumes/Data/pizlo/octonary/OpenSource/WebKitBuild/Release/jsc (
r99833
) Collected 12 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. TipOfTree DontReparse SunSpider: 3d-cube 9.1379+-0.0258 ^ 8.9776+-0.0431 ^ definitely 1.0179x faster 3d-morph 10.2370+-0.1657 10.0917+-0.0355 might be 1.0144x faster 3d-raytrace 9.6404+-0.0490 ^ 9.2559+-0.1173 ^ definitely 1.0415x faster access-binary-trees 1.9784+-0.0069 ^ 1.9054+-0.0058 ^ definitely 1.0383x faster access-fannkuch 9.3247+-0.0053 ^ 9.1539+-0.0087 ^ definitely 1.0187x faster access-nbody 5.3036+-0.0459 ^ 5.0134+-0.0038 ^ definitely 1.0579x faster access-nsieve 3.7628+-0.0067 ? 3.7635+-0.0509 ? bitops-3bit-bits-in-byte 1.5622+-0.0153 ^ 1.4952+-0.0147 ^ definitely 1.0448x faster bitops-bits-in-byte 6.1942+-0.0324 6.1408+-0.0401 bitops-bitwise-and 3.9978+-0.0078 ^ 3.9756+-0.0045 ^ definitely 1.0056x faster bitops-nsieve-bits 6.8386+-0.0425 6.8337+-0.0432 controlflow-recursive 2.8144+-0.0257 ^ 2.7221+-0.0205 ^ definitely 1.0339x faster crypto-aes 9.3584+-0.0448 ^ 8.6663+-0.0735 ^ definitely 1.0799x faster crypto-md5 3.3037+-0.0118 ^ 3.0594+-0.0265 ^ definitely 1.0799x faster crypto-sha1 3.0046+-0.0219 ^ 2.6333+-0.0255 ^ definitely 1.1410x faster date-format-tofte 13.1019+-0.1050 12.9822+-0.1197 date-format-xparb 12.6418+-0.2223 12.4466+-0.1807 might be 1.0157x faster math-cordic 9.1270+-0.3557 9.1158+-0.3626 math-partial-sums 12.6792+-0.0344 12.6775+-0.0641 math-spectral-norm 3.3260+-0.0107 ^ 3.1266+-0.0059 ^ definitely 1.0638x faster regexp-dna 16.6620+-0.1701 16.4830+-0.1113 might be 1.0109x faster string-base64 5.0312+-0.0447 ^ 4.7826+-0.0561 ^ definitely 1.0520x faster string-fasta 8.6124+-0.0827 ^ 8.4399+-0.0109 ^ definitely 1.0204x faster string-tagcloud 16.2052+-0.1513 16.1084+-0.1053 string-unpack-code 28.6954+-0.0939 ^ 27.5795+-0.0749 ^ definitely 1.0405x faster string-validate-input 6.8491+-0.0488 ? 6.9466+-0.0508 ? might be 1.0142x slower <arithmetic> * 8.4381+-0.0379 ^ 8.2453+-0.0344 ^ definitely 1.0234x faster <geometric> 6.7285+-0.0257 ^ 6.5338+-0.0271 ^ definitely 1.0298x faster <harmonic> 5.2671+-0.0166 ^ 5.0645+-0.0213 ^ definitely 1.0400x faster TipOfTree DontReparse V8: crypto 96.4900+-0.2991 ^ 94.1861+-0.2984 ^ definitely 1.0245x faster deltablue 217.5844+-0.9899 ^ 212.8585+-0.9464 ^ definitely 1.0222x faster earley-boyer 130.9492+-1.0780 ^ 127.8899+-1.0225 ^ definitely 1.0239x faster raytrace 81.0926+-0.6513 ^ 77.3365+-0.4481 ^ definitely 1.0486x faster regexp 151.3493+-0.3147 ^ 149.5843+-0.3025 ^ definitely 1.0118x faster richards 173.5795+-1.5279 ^ 168.0494+-0.3756 ^ definitely 1.0329x faster splay 108.7153+-1.4730 106.6012+-1.2274 might be 1.0198x faster <arithmetic> 137.1086+-0.5109 ^ 133.7865+-0.2295 ^ definitely 1.0248x faster <geometric> * 130.2653+-0.4899 ^ 126.9433+-0.2811 ^ definitely 1.0262x faster <harmonic> 123.8923+-0.4964 ^ 120.5107+-0.3238 ^ definitely 1.0281x faster TipOfTree DontReparse Kraken: ai-astar 896.4849+-0.6290 ? 896.7507+-0.6768 ? audio-beat-detection 256.6795+-1.3877 ? 257.9164+-3.2833 ? audio-dft 313.6312+-2.6722 ? 314.6306+-3.0242 ? audio-fft 166.9654+-0.4950 166.8619+-0.2442 audio-oscillator 350.5585+-0.7851 ! 353.6672+-2.3029 ! definitely 1.0089x slower imaging-darkroom 403.5161+-5.9836 403.3743+-5.9462 imaging-desaturate 291.2893+-0.1042 291.1998+-0.1519 imaging-gaussian-blur 750.8418+-0.1805 ? 753.1201+-3.0041 ? json-parse-financial 86.7433+-0.7667 ! 88.5801+-0.3610 ! definitely 1.0212x slower json-stringify-tinderbox 94.8303+-0.5800 ! 96.2029+-0.6412 ! definitely 1.0145x slower stanford-crypto-aes 139.4328+-0.8114 138.7555+-0.7377 stanford-crypto-ccm 136.8853+-0.4446 ! 138.3731+-0.9205 ! definitely 1.0109x slower stanford-crypto-pbkdf2 281.8990+-2.1521 280.7161+-2.1571 stanford-crypto-sha256-iterative 118.3287+-0.2864 118.2437+-0.4310 <arithmetic> * 306.2919+-0.5709 ? 307.0280+-0.7822 ? <geometric> 238.7151+-0.5075 ? 239.6552+-0.5550 ? <harmonic> 192.2048+-0.5310 ! 193.4355+-0.3946 ! definitely 1.0064x slower TipOfTree DontReparse All benchmarks: <arithmetic> 116.3242+-0.1934 115.9420+-0.2147 <geometric> 30.2882+-0.0696 ^ 29.7204+-0.0740 ^ definitely 1.0191x faster <harmonic> 9.2782+-0.0285 ^ 8.9281+-0.0367 ^ definitely 1.0392x faster TipOfTree DontReparse Geomean of preferred means: <scaled-result> 69.5663+-0.1551 ^ 68.4953+-0.1164 ^ definitely 1.0156x faster
Filip Pizlo
Comment 4
2011-11-10 03:59:00 PST
And another computer… Benchmark report for SunSpider, V8, and Kraken on nitroflex.local (MacBookPro8,2). VMs tested: "TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (
r99833
) "DontReparse" at /Volumes/Data/pizlo/octonary/OpenSource/WebKitBuild/Release/jsc (
r99833
) Collected 12 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. TipOfTree DontReparse SunSpider: 3d-cube 7.1160+-0.2107 7.0391+-0.1941 might be 1.0109x faster 3d-morph 7.4623+-0.1149 7.4498+-0.0730 3d-raytrace 7.3824+-0.1565 7.1738+-0.1668 might be 1.0291x faster access-binary-trees 1.5903+-0.0913 1.5834+-0.0779 access-fannkuch 6.3282+-0.1507 6.1782+-0.1300 might be 1.0243x faster access-nbody 3.6238+-0.0754 ^ 3.4095+-0.0862 ^ definitely 1.0629x faster access-nsieve 2.5397+-0.0578 ? 2.5805+-0.1049 ? might be 1.0161x slower bitops-3bit-bits-in-byte 1.2867+-0.0294 1.2578+-0.0243 might be 1.0230x faster bitops-bits-in-byte 2.3013+-0.0503 2.2661+-0.0535 might be 1.0155x faster bitops-bitwise-and 3.2182+-0.0962 ? 3.2186+-0.0473 ? bitops-nsieve-bits 5.1653+-0.0542 ? 5.2901+-0.0994 ? might be 1.0242x slower controlflow-recursive 2.1026+-0.0452 2.0529+-0.0553 might be 1.0243x faster crypto-aes 7.4216+-0.1925 ^ 6.9160+-0.2297 ^ definitely 1.0731x faster crypto-md5 2.5961+-0.0629 ^ 2.3522+-0.0650 ^ definitely 1.1037x faster crypto-sha1 2.3015+-0.0560 ^ 2.0612+-0.0614 ^ definitely 1.1166x faster date-format-tofte 9.9099+-0.3246 9.8278+-0.1697 date-format-xparb 9.2049+-0.2389 ? 9.6297+-0.2415 ? might be 1.0461x slower math-cordic 6.4217+-0.0867 ? 6.4958+-0.0747 ? might be 1.0115x slower math-partial-sums 7.5150+-0.1556 ? 7.5324+-0.0968 ? math-spectral-norm 2.5063+-0.0604 ^ 2.3266+-0.0575 ^ definitely 1.0773x faster regexp-dna 11.5327+-0.2318 11.5130+-0.2307 string-base64 3.9163+-0.1086 ^ 3.7127+-0.0613 ^ definitely 1.0548x faster string-fasta 6.2300+-0.1228 ? 6.3211+-0.1558 ? might be 1.0146x slower string-tagcloud 11.7260+-0.2376 11.3764+-0.2851 might be 1.0307x faster string-unpack-code 20.3561+-0.3019 19.9831+-0.4012 might be 1.0187x faster string-validate-input 5.1792+-0.1294 5.1156+-0.0853 might be 1.0124x faster <arithmetic> * 6.0359+-0.0345 ^ 5.9486+-0.0316 ^ definitely 1.0147x faster <geometric> 4.8367+-0.0247 ^ 4.7343+-0.0335 ^ definitely 1.0216x faster <harmonic> 3.8515+-0.0262 ^ 3.7419+-0.0376 ^ definitely 1.0293x faster TipOfTree DontReparse V8: crypto 71.4460+-0.3823 ^ 69.7978+-0.5671 ^ definitely 1.0236x faster deltablue 157.8425+-0.6965 ^ 154.9324+-0.8671 ^ definitely 1.0188x faster earley-boyer 87.6959+-0.7451 ^ 85.4942+-0.8234 ^ definitely 1.0258x faster raytrace 59.1921+-0.4369 ^ 56.6614+-0.5331 ^ definitely 1.0447x faster regexp 105.1183+-0.6274 ^ 103.9579+-0.3793 ^ definitely 1.0112x faster richards 122.5364+-0.3778 ^ 120.4521+-0.3004 ^ definitely 1.0173x faster splay 70.8957+-0.6718 ? 71.7627+-1.3166 ? might be 1.0122x slower <arithmetic> 96.3896+-0.1855 ^ 94.7226+-0.2461 ^ definitely 1.0176x faster <geometric> * 91.4376+-0.2092 ^ 89.7871+-0.2735 ^ definitely 1.0184x faster <harmonic> 87.0474+-0.2357 ^ 85.3657+-0.3124 ^ definitely 1.0197x faster TipOfTree DontReparse Kraken: ai-astar 488.4696+-3.7278 486.2353+-2.2241 audio-beat-detection 187.0626+-0.4844 186.4867+-0.7610 audio-dft 262.7114+-2.6286 259.0819+-2.1574 might be 1.0140x faster audio-fft 121.3754+-0.3667 ! 122.9276+-0.9519 ! definitely 1.0128x slower audio-oscillator 247.5074+-0.9471 ? 247.8469+-1.0463 ? imaging-darkroom 296.4649+-4.2409 ? 296.6776+-4.4837 ? imaging-desaturate 221.6823+-0.6434 221.5570+-0.7926 imaging-gaussian-blur 546.3750+-1.4749 544.4993+-1.1686 json-parse-financial 56.9925+-0.2459 ? 57.1872+-0.1117 ? json-stringify-tinderbox 67.6576+-0.1270 ? 67.7753+-0.5094 ? stanford-crypto-aes 94.2014+-0.4049 ? 95.0778+-0.5380 ? stanford-crypto-ccm 97.8710+-0.7897 ? 98.1923+-0.5955 ? stanford-crypto-pbkdf2 186.9708+-0.6859 ! 189.3529+-1.1652 ! definitely 1.0127x slower stanford-crypto-sha256-iterative 79.0255+-0.3167 78.6344+-0.1717 <arithmetic> * 211.0262+-0.4791 210.8237+-0.3195 <geometric> 167.4214+-0.2784 ? 167.5952+-0.2236 ? <harmonic> 134.1494+-0.1889 ? 134.4629+-0.1789 ? TipOfTree DontReparse All benchmarks: <arithmetic> 80.5538+-0.1423 ^ 80.1969+-0.1096 ^ definitely 1.0045x faster <geometric> 21.5367+-0.0629 ^ 21.2318+-0.0878 ^ definitely 1.0144x faster <harmonic> 6.7768+-0.0449 ^ 6.5878+-0.0646 ^ definitely 1.0287x faster TipOfTree DontReparse Geomean of preferred means: <scaled-result> 48.8348+-0.0937 ^ 48.2887+-0.1133 ^ definitely 1.0113x faster
Filip Pizlo
Comment 5
2011-11-10 04:03:28 PST
And another…. Benchmark report for SunSpider, V8, and Kraken on bigmac.local (MacPro5,1). VMs tested: "TipOfTree" at /Volumes/Data/pizlo/quinary/OpenSource/WebKitBuild/Release/jsc (
r99833
) "DontReparse" at /Volumes/Data/pizlo/octonary/OpenSource/WebKitBuild/Release/jsc (
r99833
) Collected 12 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. TipOfTree DontReparse SunSpider: 3d-cube 7.6214+-0.0260 ^ 7.4704+-0.0339 ^ definitely 1.0202x faster 3d-morph 8.5726+-0.2168 8.5425+-0.1181 3d-raytrace 8.0998+-0.0556 ^ 7.8017+-0.0736 ^ definitely 1.0382x faster access-binary-trees 1.6547+-0.0060 ^ 1.5997+-0.0131 ^ definitely 1.0344x faster access-fannkuch 7.7523+-0.0579 7.6534+-0.0855 might be 1.0129x faster access-nbody 4.3934+-0.0093 ^ 4.1874+-0.0198 ^ definitely 1.0492x faster access-nsieve 3.1666+-0.0123 3.1577+-0.0385 bitops-3bit-bits-in-byte 1.3018+-0.0160 ^ 1.2567+-0.0156 ^ definitely 1.0359x faster bitops-bits-in-byte 5.1806+-0.0588 ^ 5.0844+-0.0243 ^ definitely 1.0189x faster bitops-bitwise-and 3.3273+-0.0264 3.3032+-0.0160 bitops-nsieve-bits 5.6694+-0.0387 ? 5.7061+-0.0751 ? controlflow-recursive 2.3453+-0.0259 2.3022+-0.0323 might be 1.0187x faster crypto-aes 7.8643+-0.1285 ^ 7.1920+-0.0663 ^ definitely 1.0935x faster crypto-md5 2.8297+-0.0241 ^ 2.5823+-0.0402 ^ definitely 1.0958x faster crypto-sha1 2.5183+-0.0171 ^ 2.2355+-0.0192 ^ definitely 1.1265x faster date-format-tofte 10.7092+-0.0645 ? 10.9788+-0.3545 ? might be 1.0252x slower date-format-xparb 10.1238+-0.1178 ? 10.1934+-0.2675 ? math-cordic 8.0333+-0.2888 ^ 7.4030+-0.2621 ^ definitely 1.0852x faster math-partial-sums 10.5474+-0.0294 ? 10.5618+-0.0540 ? math-spectral-norm 2.7740+-0.0234 ^ 2.6236+-0.0308 ^ definitely 1.0573x faster regexp-dna 13.4838+-0.2007 13.3818+-0.1448 string-base64 4.2050+-0.1004 ^ 3.9436+-0.0208 ^ definitely 1.0663x faster string-fasta 7.1460+-0.0195 7.1042+-0.0590 string-tagcloud 13.3047+-0.0846 ? 13.3969+-0.1912 ? string-unpack-code 23.2199+-0.1831 ^ 22.3907+-0.1962 ^ definitely 1.0370x faster string-validate-input 5.7297+-0.0603 ? 5.7545+-0.0318 ? <arithmetic> * 6.9836+-0.0242 ^ 6.8387+-0.0298 ^ definitely 1.0212x faster <geometric> 5.6027+-0.0181 ^ 5.4455+-0.0190 ^ definitely 1.0289x faster <harmonic> 4.4005+-0.0171 ^ 4.2394+-0.0178 ^ definitely 1.0380x faster TipOfTree DontReparse V8: crypto 80.2850+-0.3858 ^ 78.1313+-0.2512 ^ definitely 1.0276x faster deltablue 180.4189+-0.7193 ? 180.5901+-1.6778 ? earley-boyer 109.3444+-0.6908 ^ 106.7310+-0.9064 ^ definitely 1.0245x faster raytrace 67.1571+-0.5759 ^ 63.9823+-0.3642 ^ definitely 1.0496x faster regexp 127.2048+-0.4775 ^ 125.1099+-0.4190 ^ definitely 1.0167x faster richards 143.2527+-0.4555 ^ 139.8843+-0.6572 ^ definitely 1.0241x faster splay 92.1319+-1.3306 91.2307+-1.3058 <arithmetic> 114.2564+-0.1739 ^ 112.2371+-0.3763 ^ definitely 1.0180x faster <geometric> * 108.6299+-0.2137 ^ 106.3404+-0.3306 ^ definitely 1.0215x faster <harmonic> 103.3378+-0.2485 ^ 100.7975+-0.3126 ^ definitely 1.0252x faster TipOfTree DontReparse Kraken: ai-astar 811.1446+-12.1836 ! 829.6888+-1.2981 ! definitely 1.0229x slower audio-beat-detection 210.4634+-0.3345 210.3719+-0.3645 audio-dft 267.3029+-8.5981 263.2959+-2.3874 might be 1.0152x faster audio-fft 137.5691+-0.3430 ? 137.5746+-0.3763 ? audio-oscillator 292.3153+-2.9677 290.9230+-0.8406 imaging-darkroom 334.1868+-4.7150 ? 335.8100+-4.9925 ? imaging-desaturate 241.3295+-0.2320 ? 241.5680+-0.2330 ? imaging-gaussian-blur 622.9331+-0.6125 ? 622.9645+-0.7185 ? json-parse-financial 71.5099+-0.2457 ! 72.7217+-0.2078 ! definitely 1.0169x slower json-stringify-tinderbox 78.9935+-0.5460 ! 79.9847+-0.3438 ! definitely 1.0125x slower stanford-crypto-aes 116.4337+-0.5026 115.5108+-1.2490 stanford-crypto-ccm 117.1500+-3.0983 115.0906+-0.5055 might be 1.0179x faster stanford-crypto-pbkdf2 232.4978+-0.4830 ^ 231.4326+-0.5179 ^ definitely 1.0046x faster stanford-crypto-sha256-iterative 97.9107+-0.2340 97.7429+-0.3122 <arithmetic> * 259.4100+-0.7933 ? 260.3343+-0.3323 ? <geometric> 199.7779+-0.5859 ? 199.8830+-0.2001 ? <harmonic> 160.0671+-0.5030 ? 160.2880+-0.1659 ? TipOfTree DontReparse All benchmarks: <arithmetic> 98.1513+-0.2377 98.0457+-0.1025 <geometric> 25.2640+-0.0594 ^ 24.7944+-0.0549 ^ definitely 1.0189x faster <harmonic> 7.7511+-0.0296 ^ 7.4725+-0.0307 ^ definitely 1.0373x faster TipOfTree DontReparse Geomean of preferred means: <scaled-result> 58.1661+-0.1051 ^ 57.4204+-0.1284 ^ definitely 1.0130x faster
Filip Pizlo
Comment 6
2011-11-10 04:07:25 PST
Created
attachment 114468
[details]
the patch
Filip Pizlo
Comment 7
2011-11-10 04:11:28 PST
<
rdar://problem/10425588
>
Geoffrey Garen
Comment 8
2011-11-10 12:06:47 PST
Comment on
attachment 114468
[details]
the patch View in context:
https://bugs.webkit.org/attachment.cgi?id=114468&action=review
r=me, with some comments below.
> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1655 > + if (hasInstructions() && m_shouldDiscardBytecodeLater)
The "Later" in "m_shouldDiscardBytecodeLater" confused me because it sounded like setting the flag was a way to delay or prevent discarding bytecode ("Not now, later"), but actually it's the way to cause bytecode to be discarded. I think "m_shouldDiscardBytecode" would be better.
> Source/JavaScriptCore/dfg/DFGByteCodeCache.h:68 > + // This is likely to be good enough.
Sneaky, but true. I would remove this comment, or add a "why" explanation. Thinking this through, I wonder if | might be clearer than ^ here. This hash function really just relies on the low bits of m_executable being zero, and the low bits of m_kind being meaningful.
> Source/JavaScriptCore/heap/ListableHandler.h:60 > + void addNotThreadSafe(T* handler)
Please make this private.
> Source/JavaScriptCore/heap/UltraWeakFinalizer.h:37 > +class UltraWeakFinalizer : public ListableHandler<UltraWeakFinalizer> {
The "Ultra" in "UltraWeak" didn't do enough to tell me what this class is. To my mind, two things are unique about this kind of "finalizer": (1) It fires no matter what, even if the owning object is live; (2) If GC happens soon enough, it might prefer not to fire, thus avoiding cache churn. Based on those facts, I'd suggest: - CacheOwner (with a "clearCache" callback) OR - UnconditionalFinalizer (with a "finalize" callback)
Filip Pizlo
Comment 9
2011-11-10 14:03:20 PST
> The "Later" in "m_shouldDiscardBytecodeLater" confused me because it sounded like setting the flag was a way to delay or prevent discarding bytecode ("Not now, later"), but actually it's the way to cause bytecode to be discarded. I think "m_shouldDiscardBytecode" would be better.
I dropped the Later.
> > > Source/JavaScriptCore/dfg/DFGByteCodeCache.h:68 > > + // This is likely to be good enough. > > Sneaky, but true. I would remove this comment, or add a "why" explanation. > > Thinking this through, I wonder if | might be clearer than ^ here. This hash function really just relies on the low bits of m_executable being zero, and the low bits of m_kind being meaningful.
I dropped the comment. It's actually less sneaky and even more correct: I'm using the PtrHash, which picks an appropriate int hash, so even just the pointer hash has all bits populated with some manner of rubbish. Then I xor in the mode enum, which flips the low bit. It's only "likely" to be good enough because if someone changed the hash table implementation to use high bits instead of low bits in the mask, then this would become slightly sub-optimal. But even then it would only be *slightly* sub-optimal because it would mean collisions when you want to inline an executable for both a call and a construction. And even then that collision probably won't cost you anything since the dominant cost of inlining is, well, inlining. So we're starting to enter into the unlikely * unlikely * unlikely domain of probabilities.
> > > Source/JavaScriptCore/heap/ListableHandler.h:60 > > + void addNotThreadSafe(T* handler) > > Please make this private.
I did, though probably one of my next patches will make it public again. The WeakReferenceHarvester list should be processed multiple times, and the easiest way to do that is for the GC to consume the global one into a private one, which can be added to without locking.
> > > Source/JavaScriptCore/heap/UltraWeakFinalizer.h:37 > > +class UltraWeakFinalizer : public ListableHandler<UltraWeakFinalizer> { > > The "Ultra" in "UltraWeak" didn't do enough to tell me what this class is. To my mind, two things are unique about this kind of "finalizer": (1) It fires no matter what, even if the owning object is live; (2) If GC happens soon enough, it might prefer not to fire, thus avoiding cache churn. Based on those facts, I'd suggest: > > - CacheOwner (with a "clearCache" callback) > OR > - UnconditionalFinalizer (with a "finalize" callback)
I picked UnconditionalFinalizer, and made the callback "finalizeUnconditionally", since I did not want CodeBlock to have a method called "finalize". It would be hard to tell where that method came from.
Filip Pizlo
Comment 10
2011-11-10 14:04:25 PST
Landed in
http://trac.webkit.org/changeset/99898
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug