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

internal/stack: Use control flow for state #109

Closed
wants to merge 1 commit into from
Closed

internal/stack: Use control flow for state #109

wants to merge 1 commit into from

Conversation

abhinav
Copy link
Collaborator

@abhinav abhinav commented Oct 21, 2023

In anticipation of parsing more information from stack traces make the
stack trace parsing logic more manageable by moving it from a state
machine into a layout closer to a recursive descent parser.

That is, instead of a central loop that reads input line-by-line
and needs to manage its various states:

current, result := ...
for {
    input := read()
    if cond(input) {
        result.append(current)
        current = startNew(input)
    } else {
        current = accumulate(input)
    }
}
result = flush(current)

Break it down so that parsing of individual results is its own function,
representing the state machine via control flow.

result := ...
for {
    input := read()
    if cond(input) {
        result.append(parseOne())
    }
}

// where

func parseOne(input) {
    value := ...
    for ; !cond(input); input = read() {
        value = accumulate(input)
    }
    return value
}

The net effect of this is to make the parsing logic more maintainable
once it gets more complex -- adds more states.

For example, to parse more information for individual stacks
with a state machine, we'd have to make the main loop more complex.
State for an individual stack (e.g. "all the functions in the stack")
will leak into the state management for the whole state machine.
On the other hand, with this method, we'll only modify parseStack,
keeping its responsiblity encapsulated to parsing a single stack trace.

This idea was also demonstrated recently in the first section of
Storing Data in Control flow by Russ Cox.


To make it easy to write this parser, we switch from bufio.Reader
to bufio.Scanner, and wrap it with the ability to "Unscan":
basically "don't move forward on next Scan()".

In anticipation of parsing more information from stack traces make the
stack trace parsing logic more manageable by moving it from a state
machine into a layout closer to a recursive descent parser.

That is, instead of a central loop that reads input line-by-line
and needs to manage its various states:

    current, result := ...
    for {
        input := read()
        if cond(input) {
            result.append(current)
            current = startNew(input)
        } else {
            current = accumulate(input)
        }
    }
    result = flush(current)

Break it down so that parsing of individual results is its own function,
representing the state machine via control flow.

    result := ...
    for {
        input := read()
        if cond(input) {
            result.append(parseOne())
        }
    }

    // where

    func parseOne(input) {
        value := ...
        for ; !cond(input); input = read() {
            value = accumulate(input)
        }
        return value
    }

The net effect of this is to make the parsing logic more maintainable
once it gets more complex -- adds more states.

For example, to parse more information for individual stacks
with a state machine, we'd have to make the main loop more complex.
State for an individual stack (e.g. "all the functions in the stack")
will leak into the state management for the whole state machine.
On the other hand, with this method, we'll only modify parseStack,
keeping its responsiblity encapsulated to parsing a single stack trace.

This idea was also demonstrated recently in the first section of
[Storing Data in Control flow by Russ Cox][1].

  [1]: https://research.swtch.com/pcdata#step

---

To make it easy to write this parser, we switch from bufio.Reader
to bufio.Scanner, and wrap it with the ability to "Unscan":
basically "don't move forward on next Scan()".
@abhinav abhinav closed this Oct 21, 2023
@abhinav
Copy link
Collaborator Author

abhinav commented Oct 21, 2023

Not yet ready. Need #108 to run CI against 1.20+.

@abhinav abhinav deleted the parse-flow branch October 21, 2023 20:42
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.

1 participant