Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Schema parser & printer #12

Merged
merged 7 commits into from
Oct 7, 2015
Merged

Schema parser & printer #12

merged 7 commits into from
Oct 7, 2015

Conversation

jhgg
Copy link
Member

@jhgg jhgg commented Oct 7, 2015

PR for issue #11

  • Adds the schema type definitions to ast.ast to have generate_ast() generate classes for them.
  • parse_type_definition will now parse schema types.
  • Implement parsers for each schema type.
  • Improve error message in Executor.complete_value when runtime type doesn't match.
  • Implement more missing executor tests.
  • The visit function will no longer attempt to deep copy nodes when there is an edit. Shallow copies are perfectly acceptable, since we are only overwriting attributes on edit.
  • The Visitor class now pre-calculates an AST Type to Handler map in the metaclass, allowing for a much quicker enter/leave implementation.
  • Install six.
  • Use VisitorMeta to power TypeInfo so that it's not a huge isinstance chain.

Checklist to merge:

  • Implement remaining schema parser tests.
  • Implement schema printer and tests.

* Adds the schema type definitions to ast.ast to have generate_ast() generate classes for them.
* parse_type_definition will now parse schema types.
* Implement parsers for each schema type.
* Improve error message in `Executor.complete_value` when runtime type doesn't match.
* Implement more missing executor tests.
@jhgg jhgg self-assigned this Oct 7, 2015
@jhgg jhgg added this to the v0.1 milestone Oct 7, 2015
* Implement schema parser tests
* Also implement util `ast_to_code` which will convert an AST the python code representation of the AST.
@jhgg jhgg changed the title [WIP] Schema parser [WIP] Schema parser & printer Oct 7, 2015
* The `visit` function will no longer attempt to deep copy nodes when there is an edit. Shallow copies are perfectly acceptable, since we are only overwriting attributes on edit.
* The `Visitor` class now pre-calculates an AST Type to Handler map in the metaclass, allowing for a much quicker enter/leave implementation.
* Install `six`.
* Implement remaining part of printer so that it can print Schema types.
@jhgg jhgg changed the title [WIP] Schema parser & printer Schema parser & printer Oct 7, 2015
jhgg added a commit that referenced this pull request Oct 7, 2015
@jhgg jhgg merged commit 3378679 into master Oct 7, 2015
leave = visitor.leave
enter = visitor.enter
path_pop = path.pop
ancestors_pop = ancestors.pop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use this renamed functions instead of calling directly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a slight performance gain. In a tight loop, list.pop() vs list_pop, the latter will be faster.

def foo():
    l = []
    while True:
        l.append(1)

The disassembly produces:

  2           0 BUILD_LIST               0
              3 STORE_FAST               0 (l)

  3           6 SETUP_LOOP              23 (to 32)
        >>    9 LOAD_GLOBAL              0 (True)
             12 POP_JUMP_IF_FALSE       31

  4          15 LOAD_FAST                0 (l)
             18 LOAD_ATTR                1 (append)
             21 LOAD_CONST               1 (1)
             24 CALL_FUNCTION            1
             27 POP_TOP
             28 JUMP_ABSOLUTE            9
        >>   31 POP_BLOCK
        >>   32 LOAD_CONST               0 (None)
             35 RETURN_VALUE

As for:

def foo():
    l = []
    append = l.append
    while True:
        append(1)
  2           0 BUILD_LIST               0
              3 STORE_FAST               0 (l)

  3           6 LOAD_FAST                0 (l)
              9 LOAD_ATTR                0 (append)
             12 STORE_FAST               1 (append)

  4          15 SETUP_LOOP              20 (to 38)
        >>   18 LOAD_GLOBAL              1 (True)
             21 POP_JUMP_IF_FALSE       37

  5          24 LOAD_FAST                1 (append)
             27 LOAD_CONST               1 (1)
             30 CALL_FUNCTION            1
             33 POP_TOP
             34 JUMP_ABSOLUTE           18
        >>   37 POP_BLOCK
        >>   38 LOAD_CONST               0 (None)
             41 RETURN_VALUE

Notice that in the loop body, LOAD_FAST and then LOAD_ATTR has to be called. Whereas, LOAD_FAST is only called in the second version.

It's a small performance gain. For more, check out this article: https://pymotw.com/2/dis/

@jhgg jhgg deleted the schema-parser branch October 7, 2015 20:42
@syrusakbary
Copy link
Member

Great response!

edit_offset = 0
for edit_key, edit_value in edits:
if in_array:
edit_key -= edit_offset

if in_array and edit_value is REMOVE:
node.pop(edit_key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use node_pop perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, it's not worth it. node is always changing. Also REMOVE is very rarely returned from a visitor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants