Last Comment Bug 697906 - IonMonkey: MoveResolver does not handle hairy cycles well
: IonMonkey: MoveResolver does not handle hairy cycles well
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: ---
Assigned To: general
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-27 17:48 PDT by Marty Rosenberg [:mjrosenb]
Modified: 2012-01-30 13:27 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
handle hairy cycles (183 bytes, patch)
2011-10-27 17:48 PDT, Marty Rosenberg [:mjrosenb]
no flags Details | Diff | Splinter Review
Handle hairy cycles (for real) (2.26 KB, patch)
2011-10-28 17:55 PDT, Marty Rosenberg [:mjrosenb]
dvander: review+
Details | Diff | Splinter Review

Description Marty Rosenberg [:mjrosenb] 2011-10-27 17:48:32 PDT
Created attachment 570128 [details] [diff] [review]
handle hairy cycles

If we have the set of moves:
a -> b
b -> a
b -> c

depending on the ordering of the moves, we can emit 
b -> temp
a -> b
b -> c
temp -> a
which is wrong; we end up with the original value of a in c rather than the original value of b.
This happens by first choosing |b -> a|, then choosing |a->b|, since it conflicts, and in fact forms a cycle, and since it forms a cycle, we short circuit and immediately emit the |a->b| part of the cycle, then resolving the rest of the checks, never seeing if anything conflicts with |a->b|.
The patch removes this short circuiting.

Ideally, we won't use the temp at all, and just use the copied value of b, however that will require more thorough algorithmic changes.
Comment 1 David Anderson [:dvander] 2011-10-28 15:50:12 PDT
Patch is empty - wrong file?
Comment 2 Marty Rosenberg [:mjrosenb] 2011-10-28 17:55:24 PDT
Created attachment 570435 [details] [diff] [review]
Handle hairy cycles (for real)

Note You need to log in before you can comment on or make changes to this bug.