Summary: | Add extra space to op_call and related opcodes | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Oliver Hunt <oliver> | ||||
Component: | New Bugs | Assignee: | Oliver Hunt <oliver> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | fpizlo | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Oliver Hunt
2014-02-21 14:23:16 PST
Created attachment 224908 [details]
Patch
Comment on attachment 224908 [details]
Patch
r=me
Comment on attachment 224908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224908&action=review > Source/JavaScriptCore/ChangeLog:9 > + slot to the op_call instructions, and refactoring to make simlar Nit: simlar => similar Comment on attachment 224908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224908&action=review > Source/JavaScriptCore/ChangeLog:10 > + changes easier in future. Nit: "in future" => "in the future" Committed r164503: <http://trac.webkit.org/changeset/164503> Why? Did you test performance? Inlining is based on bytecode size, and you just made everything larger. (In reply to comment #6) > Why? > > Did you test performance? Inlining is based on bytecode size, and you just made everything larger. Because call_var_args needs an extra argument, i didn't think about the performance impact because i forgot that inlining was based on # of operands rather than # of opcodes. I'll see if i can fix this, or alternatively just remove the requirement that call_var_args be the same size as all the other calls. (In reply to comment #7) > (In reply to comment #6) > > Why? > > > > Did you test performance? Inlining is based on bytecode size, and you just made everything larger. > > Because call_var_args needs an extra argument, i didn't think about the performance impact because i forgot that inlining was based on # of operands rather than # of opcodes. > > I'll see if i can fix this, or alternatively just remove the requirement that call_var_args be the same size as all the other calls. I think it's enough to just measure it and make sure that nothing broke (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > Why? > > > > > > Did you test performance? Inlining is based on bytecode size, and you just made everything larger. > > > > Because call_var_args needs an extra argument, i didn't think about the performance impact because i forgot that inlining was based on # of operands rather than # of opcodes. > > > > I'll see if i can fix this, or alternatively just remove the requirement that call_var_args be the same size as all the other calls. > > I think it's enough to just measure it and make sure that nothing broke kk (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > Why? > > > > > > Did you test performance? Inlining is based on bytecode size, and you just made everything larger. > > > > Because call_var_args needs an extra argument, i didn't think about the performance impact because i forgot that inlining was based on # of operands rather than # of opcodes. > > > > I'll see if i can fix this, or alternatively just remove the requirement that call_var_args be the same size as all the other calls. > > I think it's enough to just measure it and make sure that nothing broke I don't see a regression under jsc on my MBP. Just about to run the DRT hosted version. |