Summary: | webkit-patch optimize-baselines sometimes creates a mac-future result | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||
Component: | Tools / Tests | Assignee: | Adam Barth <abarth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | abarth, dpranke, webkit.review.bot, zimmermann | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Ojan Vafai
2012-03-09 12:17:15 PST
Here's a BaselineOptimizer unittest that hits the error case in comment 1: def test_no_add_mac_future(self): self._assertOptimization({ 'LayoutTests/platform/mac': '29a1715a6470d5dd9486a142f609708de84cdac8', 'LayoutTests/platform/win': '453e67177a75b2e79905154ece0efba6e5bfb65d', 'LayoutTests/platform/mac-snowleopard': 'c43eaeb358f49d5e835236ae23b7e49d7f2b089f', 'LayoutTests/platform/chromium-mac': 'a9ba153c700a94ae1b206d8e4a75a621a89b4554', }, { 'LayoutTests/platform/mac': '29a1715a6470d5dd9486a142f609708de84cdac8', 'LayoutTests/platform/win': '453e67177a75b2e79905154ece0efba6e5bfb65d', 'LayoutTests/platform/mac-snowleopard': 'c43eaeb358f49d5e835236ae23b7e49d7f2b089f', 'LayoutTests/platform/chromium-mac': 'a9ba153c700a94ae1b206d8e4a75a621a89b4554', }) Created attachment 131457 [details]
Patch
I'm not sure this is the best fix, but it does fix at least this one case without breaking existing tests. Comment on attachment 131457 [details]
Patch
I'm inclined to r- this because I don't think we should have any code anywhere that is referencing directories that shouldn't actually exist. I realize you didn't create this problem, but I think this is just masking the fact that there's an underlying bug, not fixing the underlying bug.
On the other hand, I could be open to landing this as a workaround for now (since you're presumably blocked on this) and filing another bug to reference this chunk of code to fix it properly.
Adam, WDYT?
Comment on attachment 131457 [details]
Patch
This is a fine workaround. Another thing you can do is to reject the new_results_by_directory in optimize() if it contains any virtual directories. That essentially means that the optimization algorithm failed, which happens sometimes.
(In reply to comment #6) > (From update of attachment 131457 [details]) > This is a fine workaround. Another thing you can do is to reject the new_results_by_directory in optimize() if it contains any virtual directories. That essentially means that the optimization algorithm failed, which happens sometimes. I tried that first. I think this is better because there are cases where we will successfully optimize. Oh, I meant we could do both. This patch is fine as-is though. Comment on attachment 131457 [details] Patch Clearing flags on attachment: 131457 Committed r110533: <http://trac.webkit.org/changeset/110533> All reviewed patches have been landed. Closing bug. |