Skip to content

Make implicit boolean conversions explicit #106008

Description

@brandtbucher

...as discussed in faster-cpython/ideas#568.

By adding a dedicated instruction for converting values to bool, we can easily specialize the conditions of all remaining branches in the bytecode while keeping the branches themselves as "dumb" and simple as possible.

This is one of the few remaining common cases where a little specilization will give us a lot of useful information (such as "this branch won't execute arbitrary code") for higher tiers of optimization. Plus, the most common specializations (such as for bool or None) will be effectively no-ops.

Linked PRs

Activity

  1. added
    performancePerformance or resource usage
    interpreter-core(Objects, Python, Grammar, and Parser dirs)
    3.13only security fixes
    on Jun 23, 2023
  2. self-assigned this
    on Jun 23, 2023
  3. added a commit that references this issue on Jun 29, 2023
  4. pablogsal commented on Jul 3, 2023

    @pablogsal
    Member

    @brandtbucher Apparently this PR introduced a refleak in test_grammar:

    Raised RLIMIT_NOFILE: 256 -> 1024
    0:00:00 load avg: 14.52 Run tests sequentially
    0:00:00 load avg: 14.52 [1/1] test_grammar
    beginning 9 repetitions
    123456789
    .........
    test_grammar leaked [2, 2, 2, 2] references, sum=8
    test_grammar leaked [2, 2, 2, 2] memory blocks, sum=8
    test_grammar failed (reference leak)
    
    == Tests result: FAILURE ==
    
    1 test failed:
        test_grammar
    
    Total duration: 349 ms
    Tests result: FAILURE
    

    The previous commit builds correctly:

    ❯ git checkout HEAD^
    Previous HEAD position was 7b2d94d8751 GH-106008: Make implicit boolean conversions explicit (GH-106003)
    HEAD is now at 6e9f83d9aee GH-106250: Support insts using one cache entry and no oparg (GH-106252)
    
    ❯ make -j -s
    ...
    
    ❯ ./python.exe -m test test_grammar -R :
    Raised RLIMIT_NOFILE: 256 -> 1024
    0:00:00 load avg: 11.57 Run tests sequentially
    0:00:00 load avg: 11.57 [1/1] test_grammar
    beginning 9 repetitions
    123456789
    .........
    
    == Tests result: SUCCESS ==
    
    1 test OK.
    
    Total duration: 342 ms
    Tests result: SUCCESS
    

    can you take a look?

  5. brandtbucher commented on Jul 3, 2023

    @brandtbucher
    MemberAuthor

    Huh. Yeah, I'll take a look.

  6. brandtbucher commented on Jul 3, 2023

    @brandtbucher
    MemberAuthor

    Ah, I think I found it.

  7. added a commit that references this issue on Jul 4, 2023
  8. added a commit that references this issue on Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.13only security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)performancePerformance or resource usage

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions