[engine] Convert all isinstance product checks to using Exactly type constraints

Review Request #4236 - Created Sept. 13, 2016 and submitted

Information
Nick Howard (Twitter)
pants
3868
4233
Reviewers
pants-reviews
kwlzn, stuhood, yujiec

The existing type checking required coercion rules in order to ensure that product types that are produced by rules that declare they produce their super class would work.

This patch removes the sub/superclassing in favor of using the Exactly type constraint for type checking.

To do so, it adds a constraint method on SymbolTables that returns a type constraint defining what types can come out of using the symbol table. That constraint is then used in rule declarations that either produce or consume parsed products.

Other elements
- TaskNode now contains the rule it represents. That way it can use the rule components without recreating them and its repr now contains the rule's repr.
- collect_item_of_type is inlined back into SelectNode#select_literal. Removing the coercion rule class made extracting this unnecessary
- settled on input_selectors as the name for the collection of selectors used by a rule
- removed super / sub type checking in rule validation as it is no longer necessary
- renamed and moved intrinsic node factories to the rule module

I've been testing with the engine tests locally. Additionally I've been profiling running ./pants list :: with the engine enabled. It's slightly slower right now, but the removal of the coercion rules reduced the number of nodes in the graph by 5%. With further optimization of scheduling this enables, I think we can get to a better place.

(master)$ for i in 1 2 3; do ./pants --enable-v2-engine list :: -ldebug 2>&1 | grep 'scheduling iterations'; done
DEBUG] ran 152 scheduling iterations, 21135 runnables, and 62902 steps in 6.560897 seconds. there are 39158 total nodes.
DEBUG] ran 2 scheduling iterations, 2 runnables, and 23 steps in 0.001659 seconds. there are 39173 total nodes.
DEBUG] ran 0 scheduling iterations, 0 runnables, and 0 steps in 0.000002 seconds. there are 39173 total nodes.
DEBUG] ran 152 scheduling iterations, 21135 runnables, and 62902 steps in 6.554824 seconds. there are 39158 total nodes.
DEBUG] ran 2 scheduling iterations, 2 runnables, and 23 steps in 0.001486 seconds. there are 39173 total nodes.
DEBUG] ran 0 scheduling iterations, 0 runnables, and 0 steps in 0.000001 seconds. there are 39173 total nodes.
DEBUG] ran 152 scheduling iterations, 21135 runnables, and 62902 steps in 6.748163 seconds. there are 39158 total nodes.
DEBUG] ran 2 scheduling iterations, 2 runnables, and 23 steps in 0.001648 seconds. there are 39173 total nodes.
DEBUG] ran 0 scheduling iterations, 0 runnables, and 0 steps in 0.000002 seconds. there are 39173 total nodes.
$ git co nhoward/typechecks_for_everyone 
Switched to branch 'nhoward/typechecks_for_everyone'
Your branch is up-to-date with 'origin/nhoward/typechecks_for_everyone'.
(nhoward/typechecks_for_everyone)$ for i in 1 2 3; do ./pants --enable-v2-engine list :: -ldebug 2>&1 | grep 'scheduling iterations'; done
DEBUG] ran 151 scheduling iterations, 19791 runnables, and 60214 steps in 7.202758 seconds. there are 37814 total nodes.
DEBUG] ran 2 scheduling iterations, 2 runnables, and 23 steps in 0.001512 seconds. there are 37829 total nodes.
DEBUG] ran 0 scheduling iterations, 0 runnables, and 0 steps in 0.000002 seconds. there are 37829 total nodes.
DEBUG] ran 151 scheduling iterations, 19791 runnables, and 60214 steps in 7.058010 seconds. there are 37814 total nodes.
DEBUG] ran 2 scheduling iterations, 2 runnables, and 23 steps in 0.001441 seconds. there are 37829 total nodes.
DEBUG] ran 0 scheduling iterations, 0 runnables, and 0 steps in 0.000002 seconds. there are 37829 total nodes.
DEBUG] ran 151 scheduling iterations, 19791 runnables, and 60214 steps in 7.324950 seconds. there are 37814 total nodes.
DEBUG] ran 2 scheduling iterations, 2 runnables, and 23 steps in 0.001683 seconds. there are 37829 total nodes.
DEBUG] ran 0 scheduling iterations, 0 runnables, and 0 steps in 0.000004 seconds. there are 37829 total nodes.

Issues

  • 1
  • 1
  • 0
  • 2
Description From Last Updated
This use of Exactly feels awkward... sorry, know you asked for my comments yesterday. I'm not usually one to use ... Stu Hood Stu Hood
Stu Hood
Stu Hood
Nick Howard (Twitter)
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as https://github.com/pantsbuild/pants/commit/c96d44b85e41fdb14c6495021ef06b4dd26256a1
Loading...