RESOLVED LATER 15972
Add BinaryOperatorNode class to clean up the Node tree
https://bugs.webkit.org/show_bug.cgi?id=15972
Summary Add BinaryOperatorNode class to clean up the Node tree
Eric Seidel (no email)
Reported 2007-11-13 14:51:45 PST
Add BinaryOperatorNode class to clean up the Node tree
Attachments
cleanup patch (56.40 KB, patch)
2007-11-13 14:52 PST, Eric Seidel (no email)
no flags
improved patch, SunSpider reports wild results (81.60 KB, patch)
2007-11-13 17:34 PST, Eric Seidel (no email)
no flags
Eric Seidel (no email)
Comment 1 2007-11-13 14:52:03 PST
Created attachment 17241 [details] cleanup patch
Geoffrey Garen
Comment 2 2007-11-13 16:42:58 PST
Comment on attachment 17241 [details] cleanup patch operatorAsString should be KJS_FAST_CALL. Looks good. r=me if the SunSpider gods do not disapprove.
Eric Seidel (no email)
Comment 3 2007-11-13 17:34:37 PST
Created attachment 17248 [details] improved patch, SunSpider reports wild results SunSpider reports absolutely wild results from this: TEST COMPARISON FROM TO DETAILS ============================================================================= ** TOTAL **: 2.0% *slower* 4372.2ms +/- 0.2% 4460.8ms +/- 0.3% significant ============================================================================= 3d: 1.4% *slower* 494.2ms +/- 0.3% 501.2ms +/- 1.0% significant cube: ?? 166.4ms +/- 0.7% 168.2ms +/- 3.2% not conclusive: might be 1.1% *slower* morph: ?? 160.0ms +/- 0.5% 160.4ms +/- 0.4% not conclusive: might be 0.3% *slower* raytrace: 2.9% *slower* 167.8ms +/- 0.3% 172.6ms +/- 0.6% significant access: - 713.4ms +/- 0.2% 712.8ms +/- 0.5% binary-trees: 1.3% *slower* 108.8ms +/- 0.5% 110.2ms +/- 0.9% significant fannkuch: 1.0% faster 344.8ms +/- 0.2% 341.4ms +/- 0.6% significant nbody: ?? 181.6ms +/- 0.4% 182.4ms +/- 0.4% not conclusive: might be 0.4% *slower* nsieve: ?? 78.2ms +/- 0.7% 78.8ms +/- 0.7% not conclusive: might be 0.8% *slower* bitops: ?? 618.0ms +/- 0.1% 618.8ms +/- 0.2% not conclusive: might be 0.1% *slower* 3bit-bits-in-byte: 1.9% *slower* 114.2ms +/- 0.9% 116.4ms +/- 0.6% significant bits-in-byte: 0.7% *slower* 152.6ms +/- 0.4% 153.6ms +/- 0.4% significant bitwise-and: - 214.4ms +/- 0.3% 214.4ms +/- 0.3% nsieve-bits: 1.8% faster 136.8ms +/- 0.4% 134.4ms +/- 0.5% significant controlflow: ?? 154.6ms +/- 0.4% 155.2ms +/- 0.4% not conclusive: might be 0.4% *slower* recursive: ?? 154.6ms +/- 0.4% 155.2ms +/- 0.4% not conclusive: might be 0.4% *slower* crypto: - 343.8ms +/- 0.7% 343.6ms +/- 0.2% aes: 0.8% faster 97.8ms +/- 0.6% 97.0ms +/- 0.0% significant md5: 0.5% *slower* 123.4ms +/- 0.6% 124.0ms +/- 0.0% significant sha1: - 122.6ms +/- 1.4% 122.6ms +/- 0.6% date: 1.9% *slower* 364.2ms +/- 0.2% 371.0ms +/- 0.2% significant format-tofte: 1.4% *slower* 160.8ms +/- 0.3% 163.0ms +/- 0.5% significant format-xparb: 2.3% *slower* 203.4ms +/- 0.3% 208.0ms +/- 0.4% significant math: - 524.4ms +/- 0.4% 522.6ms +/- 0.6% cordic: 1.0% faster 229.4ms +/- 0.5% 227.0ms +/- 1.4% significant partial-sums: - 173.0ms +/- 0.5% 173.0ms +/- 0.5% spectral-norm: ?? 122.0ms +/- 0.7% 122.6ms +/- 0.6% not conclusive: might be 0.5% *slower* regexp: 23.4% *slower* 295.4ms +/- 0.2% 364.4ms +/- 1.9% significant dna: 23.4% *slower* 295.4ms +/- 0.2% 364.4ms +/- 1.9% significant string: 0.8% *slower* 864.2ms +/- 0.3% 871.2ms +/- 0.3% significant base64: 1.8% *slower* 120.0ms +/- 0.7% 122.2ms +/- 0.5% significant fasta: ?? 205.6ms +/- 0.5% 206.4ms +/- 0.3% not conclusive: might be 0.4% *slower* tagcloud: 1.0% *slower* 197.0ms +/- 0.4% 199.0ms +/- 0.4% significant unpack-code: 1.8% *slower* 202.0ms +/- 0.4% 205.6ms +/- 0.5% significant validate-input: 1.1% faster 139.6ms +/- 1.0% 138.0ms +/- 0.6% significant
Eric Seidel (no email)
Comment 4 2007-11-13 17:41:14 PST
Looking at the time samples for --shark --tests dna, there is very very little difference. Certainly nothing that could account for 23%. I guess the next step is to try an cache-miss session and see if there is a difference there.
Eric Seidel (no email)
Comment 5 2007-11-14 11:31:39 PST
I added --shark-cache (bug 15983) to check for level 2 cache misses, and also didn't see a difference. I'm really sad about this mystery code-gen regression. :( Especially since this kinda blocks other nice work like bug 15973.
Eric Seidel (no email)
Comment 6 2007-11-19 05:40:32 PST
This mystery regression may not be there next time we try. We'll just keep this patch around until we understand why it regresses. :)
Eric Seidel (no email)
Comment 7 2007-12-23 22:06:14 PST
Huzzah! I updated this patch to TOT, and now it's still showing slowness. but it's showing slowness in areas that it actually could have changed (like math-heavy tests). Investigating.
Eric Seidel (no email)
Comment 8 2008-01-13 14:53:50 PST
I've given up on this patch. Closing.
Note You need to log in before you can comment on or make changes to this bug.