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

Optimize various switch statements #1044

Merged
merged 11 commits into from Aug 24, 2021

Conversation

jridgewell
Copy link
Collaborator

@jridgewell jridgewell commented Aug 12, 2021

This implements:

  1. Full case folding for equivalent branches
  2. Full removal of empty tail branches (even when default isn't last branch, as long as default falls through to the empty tail)
  3. Converts 1-case switches into an If statement
  4. Converts 2-case switches into an If-Else statement
  5. Folds side-effect free case expressions into default (when there are no side-effect case expressions after them)
  6. Full Switch removal when all cases are "default"

The Switch removal is hard to describe, but easy to demonstrate:

switch (expression) {
  case 1:
  case 2:
  case 3:
  default:
    doSomething();
}

Because all of the case expressions are side-effect free, we can consider just the case bodies. Because all cases fall through to the last branch, we can check to see if this branch is always guaranteed to run due to a default branch. If everything is good, we can eliminate the switch statement entirely.

// output
doSomething();

I've hit this a few times when programming TypeScript. I tend to write exhaustive type checking, and a times that leads to code like:

switch (type) {
  case "Foo":
  case "Bar":
    if (process.env.NODE_ENV !== 'production') {
      throw new Error('invariant failed!');
    }
  default:
    doSomething(type);
}

In production builds, I I strip the invariant checks out for file size (my tests should have caught if it were ever possible to hit those cases). And this used to leave me with large useless switch statements:

switch (type) {
  case "Foo":
  case "Bar":
  default:
    doSomething(type);
}

This creates the "all cases are default" behavior, which I'd really like to have minimized.

1. This implements full case folding for equivalent branches
2. This implements default collapsing when all cases are "default"

The default collapsing is hard to describe, but easy to demonstrate:

```js
switch (expression) {
  case 1:
  case 2:
  case 3:
  default:
    doSomething();
}
```

Because all of the case `expression`s are side-effect free, we can consider just the case bodies. Because all cases fall through to the last branch, we
can check to see if this branch is always guaranteed to run due to a `default` branch. If everything is good, we can eliminate the switch statement entirely.

I've hit this a few times when programming TypeScript. I tend to write exhaustive type checking, and a times that leads to code like:

```js
switch (type) {
  case "Foo":
  case "Bar":
    if (process.env.NODE_ENV !== 'production') {
      throw new Error('invariant failed!');
    }
  default:
    doSomething(type);
}
```

In production builds, I I strip the invariant checks out for file size (my tests should have caught if it were ever possible to hit those cases). And this used to leave me with large useless switch statements:

```js
switch (type) {
  case "Foo":
  case "Bar":
  default:
    doSomething(type);
}
```

This creates the "all cases are default" behavior, which I'd really like to have minimized.
@fabiosantoscode
Copy link
Collaborator

This is a big one :)

if (default_body_index > side_effect_index) {
let prev_body_index = default_index - 1;
for (; prev_body_index >= 0; prev_body_index--) {
if (body[prev_body_index].body.length > 0) break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (body[prev_body_index].body.length > 0) break;
const maybe_inert_branch = body[prev_body_index];
if (aborts(maybe_inert_branch) || maybe_inert_branch.has_side_effects(compressor)) break;

If I correctly understand what you're doing here, this should match more "empty" branches safely.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're correct, and good idea.

This can actually be expanded to the "trim empty last branches", the forward direction when searching for the default branch's body in "trim side effect free branches falling into default", and finding the "default body" in "gut entire switch". I've added test cases for those.

@fabiosantoscode
Copy link
Collaborator

This looks really good!

However, I would like for you to add some tests. I've written a few myself, and I think this reasonably covers what you've made, but feel free to add more where you feel it's not enough.

You can either pull the jridgewell-switch-case from this repo or copy and paste the below tests at the end of test/compress/switch.js:

collapse_same_branches: {
    options = {
        switches: true,
        dead_code: true
    }
    input: {
        switch (id(1)) {
            case 1:
                console.log("PASS");
                break
            
            case 2: 
                console.log("PASS");
                break
            
        }
    }
    expect: {
        switch (id(1)) {
            case 1:
            case 2: 
                console.log("PASS");
        }
    }
    expect_stdout: "PASS"
}

// Not when the branches are break-less
collapse_same_branches_2: {
    options = {
        switches: true,
        dead_code: true
    }
    input: {
        switch (id(1)) {
            case 1:
                console.log("PASS");
            
            case 2: 
                console.log("PASS");
        }
    }
    expect: {
        switch (id(1)) {
            case 1:
                console.log("PASS");
            
            case 2: 
                console.log("PASS");
        }
    }
    expect_stdout: ["PASS", "PASS"]
}

// Empty branches at the end of the switch get trimmed
trim_empty_last_branches: {
    options = {
        switches: true,
        dead_code: true
    }
    input: {
        switch (id(1)) {
            case 1:
                console.log("PASS")
            case 2:
                // break should be removed too
                break
            case 3: {}
            case 4: 
        }
    }
    expect: {
        if (1 === id(1)) console.log("PASS")
    }
    expect_stdout: "PASS"
}

// ... But break should be kept if we're breaking to somewhere else
trim_empty_last_branches_2: {
    options = {
        switches: true,
        dead_code: true
    }
    input: {
        somewhere_else: if (id(true)) {
            switch (id(1)) {
                case 1:
                    console.log("PASS")
                case 2:
                    break somewhere_else
                case 3: {}
                case 4: 
            }
        }
    }
    expect: {
        somewhere_else: if (id(true))
            switch (id(1)) {
                case 1:
                    console.log("PASS")
                case 2:
                    break somewhere_else
            }
    }
    expect_stdout: "PASS"
}

trim_side_effect_free_branches_falling_into_default: {
    options = {
        switches: true,
        dead_code: true
    }
    input: {
        switch (id(1)) {
            case 0:
                "no side effect"
            case 1:
                // Not here either
            default:
                console.log("PASS default")
            case 2:
                console.log("PASS 2")
        }
    }
    expect: {
        if (2 !== id(1))
            console.log("PASS default");
        console.log("PASS 2")
    }
}

gut_entire_switch: {
    options = {
        switches: true,
        dead_code: true
    }
    input: {
        switch (id(123)) {
            case 1:
            case 2:
            case 3:
            default:
                console.log("PASS");
        }
    }
    expect: {
        id(123); console.log("PASS");
    }
    expect_stdout: "PASS"
}

turn_into_if: {
    options = {
        switches: true,
        dead_code: true
    }
    input: {
        switch (id(1)) {
            case id(2):
                console.log("FAIL");
        }
        console.log("PASS");
    }
    expect: {
        if (id(1) === id(2)) console.log("FAIL");
        console.log("PASS");
    }
    expect_stdout: "PASS"
}

turn_into_if_2: {
    options = {
        switches: true,
        dead_code: true
    }
    input: {
        switch (id(1)) {
            case id(2):
                console.log("FAIL");
            default:
                console.log("PASS");
        }
    }
    expect: {
        if (id(1) === id(2)) console.log("FAIL");
        console.log("PASS");
    }
    expect_stdout: "PASS"
}

fabiosantoscode and others added 3 commits August 23, 2021 16:09
The branch can't be the default, because it would have already been handled by the switch removal code.
@jridgewell
Copy link
Collaborator Author

@fabiosantoscode: Cherry-picked your test cases, and added a few more for intert bodies.

@fabiosantoscode
Copy link
Collaborator

fabiosantoscode commented Aug 24, 2021

Looking really pog!

I had to release without this, because releasing smaller changes is always better, so this is going to the next release

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

Successfully merging this pull request may close these issues.

None yet

2 participants