Summary: | jsRegExpCompile should not add implicit non-capturing bracket | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
Component: | JavaScriptCore | Assignee: | Darin Adler <darin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, ggaren | ||||||
Priority: | P2 | ||||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 16186 | ||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2007-11-29 04:07:16 PST
To fix this would likely require breaking out match() into match() and recursive_match() and making sure that only outer match() function is ever called by callers, and that recursive_match() is only ever called from match(). I expect this would fall into the "softball" category of JS speedups. So I misunderstood this bug and added code so that jsRegExpCompile will omit the outer bracket entirely when it's not needed. This doesn't help the regexp-dna test in SunSpider much at all, because almost all its regular expressions have "|" characters in them, which means they *do* require the outer bracket. Hence Eric's other suggestion, of optimizing match for this, really turns into a sort of "unrolling" of the match function, which could indeed be helpful for simple regular expressions. I can't help thinking it's a little bit too specific to this particular test, though. The more I look at the profile the more I realize it's a little bit too biased toward this one particular set of regular expressions. Created attachment 17628 [details]
patch to omit the outer bracket in cases where it's not needed
Created attachment 17768 [details]
patch
Comment on attachment 17768 [details]
patch
r=me
Committed revision 28525. |