Clean up implication logic and do more caching #2530
Conversation
hermanventer
added
the
WIP
|
Looking good so far! The internal React www bundle passes and we actually inline 25% more nodes, which is very impressive (at the cost of 25% more code). However, the internal React Native bundle still seems to hang with this PR. As you probably already see, the serializer tests seem to time out on CI with |
hermanventer
force-pushed
the
simplifier
branch
from
46d7990
to
ab8997e
Compare
hermanventer
changed the title
hermanventer
force-pushed
the
simplifier
branch
from
ab8997e
to
8d5be6d
Compare
There was a problem hiding this comment.
hermanventer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
hermanventer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
hermanventer
force-pushed
the
simplifier
branch
from
8d5be6d
to
264bb22
Compare
There was a problem hiding this comment.
hermanventer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
| @@ -103,30 +175,31 @@ export class PathConditionsImplementation extends PathConditions { | |||
| this._baseConditions = undefined; | |||
| for (let assumedCondition of savedBaseConditions._assumedConditions) { | |||
| if (assumedCondition.kind === "||") { | |||
| if (++total > 4) break; | |||
There was a problem hiding this comment.
How about making these things configurable?
| @@ -1,6 +1,6 @@ | |||
| function fn(x) { | |||
| for ( | |||
| var _iterator = x.items, | |||
| var _iterator = x.entries(), | |||
There was a problem hiding this comment.
Why this change in an existing test?
There was a problem hiding this comment.
I just noticed that the test is actually broken and the intent is very clearly x.entries().
hermanventer
removed
the
WIP
Release note: none
This attempts to reduce exponential blowup in the simplifier by doing more caching while computing implications and by imposing a depth limit on the number of simplify and implies calls.
It also tries to make implies and impliesNot more symmetrical, so that things are less ad-hoc.