addresses issue #310 #311
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| package quamina | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "math/rand" | ||
| "strings" | ||
| ) | ||
|
|
||
| // printer is an interface used to generate representations of Quamina data structures to facilitate | ||
| // debugging and optimization. It's an interface rather than a type so that a null implementation can | ||
| // be provided for production that should incur very little performance cost. | ||
| type printer interface { | ||
|
There was a problem hiding this comment. I'd suggest following what Google did when they developed the There was a problem hiding this comment. tl;dr // inspired by https://pkg.go.dev/golang.org/x/exp/slog#Logger
type Handler interface {
Enabled() bool
Handle(...)
}
type Printer struct {
handler handler
}
func New() *Printer {...}There was a problem hiding this comment. Hmm, looking at this. Unless you're actively testing/debugging and using case shellStyleType:
newFA, nextField = makeShellStyleAutomaton(valBytes, &nullPrinter{})It dawns on me that if I put a global variable in var sharedNullPrinter = &nullPrinter{}Then that line in case shellStyleType:
newFA, nextField = makeShellStyleAutomaton(valBytes, sharedNullPrinter)Then there would be no allocations ever outside of when you're testing/debugging. Would that address your concern? There was a problem hiding this comment. Also, I agree about documenting this stuff. Probably best to create a There was a problem hiding this comment. If this implementation is purely for developing, There was a problem hiding this comment. Left a comment in the place where I think alloc might happen? Ideally, we'd have a benchmark which shows the before/after here? There was a problem hiding this comment. Can't see alloc changes in this though: https://github.com/timbray/quamina/actions/runs/9324906941?pr=311 There was a problem hiding this comment. Yes, CONTRIBUTING is the place. There was a problem hiding this comment. I added a description to CONTRIBUTING in the latest commit. If that's OK I think we can resolve this conversation? |
||
| labelTable(table *smallTable, label string) | ||
| printNFA(table *smallTable) string | ||
| shortPrintNFA(table *smallTable) string | ||
| } | ||
|
|
||
| // nullPrinter is what the name says, a do-nothing implementation of the printer interface which ideally | ||
| // should consume close to zero CPU cycles. | ||
| type nullPrinter struct{} | ||
|
|
||
| const noPP = "prettyprinting not enabled" | ||
|
|
||
| func (*nullPrinter) labelTable(_ *smallTable, _ string) { | ||
| } | ||
| func (*nullPrinter) printNFA(_ *smallTable) string { | ||
| return noPP | ||
| } | ||
| func (*nullPrinter) shortPrintNFA(_ *smallTable) string { | ||
| return noPP | ||
| } | ||
|
|
||
| var sharedNullPrinter = &nullPrinter{} | ||
|
|
||
| // prettyPrinter makes a human-readable representation of a NFA; each smallTable may be | ||
| // given a label and as a side effect will get a random 3-digit serial number. For an example | ||
| // of the output, see the functions TestPP and TestNullPP in prettyprinter_test.go | ||
| type prettyPrinter struct { | ||
| randInts rand.Source | ||
| tableLabels map[*smallTable]string | ||
| tableSerials map[*smallTable]uint | ||
| } | ||
|
|
||
| func newPrettyPrinter(seed int) *prettyPrinter { | ||
| return &prettyPrinter{ | ||
| randInts: rand.NewSource(int64(seed)), | ||
| tableLabels: make(map[*smallTable]string), | ||
| tableSerials: make(map[*smallTable]uint), | ||
| } | ||
| } | ||
|
|
||
| func (pp *prettyPrinter) tableSerial(t *smallTable) uint { | ||
| return pp.tableSerials[t] | ||
| } | ||
| func (pp *prettyPrinter) tableLabel(t *smallTable) string { | ||
| return pp.tableLabels[t] | ||
| } | ||
|
|
||
| func (pp *prettyPrinter) labelTable(table *smallTable, label string) { | ||
| pp.tableLabels[table] = label | ||
| pp.tableSerials[table] = uint(pp.randInts.Int63()%500 + 500) | ||
| } | ||
|
|
||
| func (pp *prettyPrinter) printNFA(t *smallTable) string { | ||
| return pp.printNFAStep(&faState{table: t}, 0, make(map[*smallTable]bool)) | ||
| } | ||
|
|
||
| func (pp *prettyPrinter) printNFAStep(fas *faState, indent int, already map[*smallTable]bool) string { | ||
| t := fas.table | ||
| trailer := "\n" | ||
| if len(fas.fieldTransitions) != 0 { | ||
| trailer = fmt.Sprintf(" [%d transition(s)]\n", len(fas.fieldTransitions)) | ||
| } | ||
| s := " " + pp.printTable(t) + trailer | ||
| for _, step := range t.steps { | ||
| if step != nil { | ||
| for _, state := range step.steps { | ||
| _, ok := already[state.table] | ||
| if !ok { | ||
| already[state.table] = true | ||
| s += pp.printNFAStep(state, indent+1, already) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return s | ||
| } | ||
|
|
||
| func (pp *prettyPrinter) printTable(t *smallTable) string { | ||
| // going to build a string rep of a smallTable based on the unpacked form | ||
| // each line is going to be a range like | ||
| // 'c' .. 'e' => %X | ||
| // lines where the *faNext is nil are omitted | ||
| var rows []string | ||
| unpacked := unpackTable(t) | ||
|
|
||
| var rangeStart int | ||
| var b int | ||
|
|
||
| defTrans := unpacked[0] | ||
|
|
||
| for { | ||
| for b < len(unpacked) && unpacked[b] == nil { | ||
| b++ | ||
| } | ||
| if b == len(unpacked) { | ||
| break | ||
| } | ||
| rangeStart = b | ||
| lastN := unpacked[b] | ||
| for b < len(unpacked) && unpacked[b] == lastN { | ||
| b++ | ||
| } | ||
| if lastN != defTrans { | ||
| row := "" | ||
| if b == rangeStart+1 { | ||
| row += fmt.Sprintf("'%s'", branchChar(byte(rangeStart))) | ||
| } else { | ||
| row += fmt.Sprintf("'%s'…'%s'", branchChar(byte(rangeStart)), branchChar(byte(b-1))) | ||
| } | ||
| row += " → " + pp.nextString(lastN) | ||
| rows = append(rows, row) | ||
| } | ||
| } | ||
| serial := pp.tableSerial(t) | ||
| label := pp.tableLabel(t) | ||
| if defTrans != nil { | ||
| dtString := "★ → " + pp.nextString(defTrans) | ||
| return fmt.Sprintf("%d [%s] ", serial, label) + strings.Join(rows, " / ") + " / " + dtString | ||
| } else { | ||
| return fmt.Sprintf("%d [%s] ", serial, label) + strings.Join(rows, " / ") | ||
| } | ||
| } | ||
|
|
||
| func (pp *prettyPrinter) nextString(n *faNext) string { | ||
| var snames []string | ||
| for _, step := range n.steps { | ||
| snames = append(snames, fmt.Sprintf("%d %s", | ||
| pp.tableSerial(step.table), pp.tableLabel(step.table))) | ||
| } | ||
| return "[" + strings.Join(snames, " · ") + "]" | ||
| } | ||
|
|
||
| func branchChar(b byte) string { | ||
| switch b { | ||
| // TODO: Figure out how to test commented-out cases | ||
| // case 0: | ||
| // return "∅" | ||
| case valueTerminator: | ||
| return "ℵ" | ||
| // case byte(byteCeiling): | ||
| // return "♾️" | ||
| default: | ||
| return fmt.Sprintf("%c", b) | ||
| } | ||
| } | ||
|
|
||
| func (pp *prettyPrinter) shortPrintNFA(table *smallTable) string { | ||
| return fmt.Sprintf("%d-%s", pp.tableSerials[table], pp.tableLabels[table]) | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great section! Since we talked about OS env vars, how would a dev enable the pretty printer easily? Would be nice to explain this here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. CONTRIBUTING now contains a mini developer's user guide.