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

Possibly remove ShortcutTransition #20

Open
timbray opened this issue Aug 3, 2022 · 3 comments
Open

Possibly remove ShortcutTransition #20

timbray opened this issue Aug 3, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@timbray
Copy link
Collaborator

timbray commented Aug 3, 2022

If you look at Quamina's equivalent of ByteMachine you see the following fields:

	startDfa            *smallTable[*dfaStep]
	singletonMatch      []byte
	singletonTransition *fieldMatcher

The idea is that if there is only one possible value for a field, you don't even make a machine, you just put the value in singletonMatch and add transition target for it. In my measurements, this turns out to be a common occurrence. The traversal logic is easy:

	case fields.singletonMatch != nil:
		// if there's a singleton entry here, we either match the val or we're
		// done. Note: We have to check this first because addTransition might be
		// busy constructing an automaton, but it's not ready for use yet.  When
		// it's done it'll zero out the singletonMatch
		if bytes.Equal(fields.singletonMatch, val) {
			transitions = append(transitions, fields.singletonTransition)
		}
		return transitions

The reason why this might be a good idea is that when I was measuring things, ShortcutTransition was almost always at the first state of the machine. The conclusion is that a field with just one possible value is common, but with multiple values that have a common suffix is kind of rare. Possibly wildcards will change that? Anyhow, the SingletonTransition really adds a lot of work to AddRule and DeleteRule and I suspect isn't offering any more benefit than Quamina's approach.

@timbray timbray added bug Something isn't working and removed bug Something isn't working labels Aug 3, 2022
@baldawar baldawar added enhancement New feature or request good first issue Good for newcomers labels Aug 9, 2022
@baldawar
Copy link
Collaborator

baldawar commented Aug 9, 2022

That's a neat call-out. Makes sense to me given than in most cases we don't need to match more than one possible value. I'll mark it a enhancement for now. Also good first issue someone to grab in case someone wants to take this soon.

@timbray
Copy link
Collaborator Author

timbray commented Aug 9, 2022

Heh, not sure I agree on good 1st issue, you have to understand how ByteMachine fits together.

@baldawar
Copy link
Collaborator

baldawar commented Aug 9, 2022

Fair point. Let me take that out.

@baldawar baldawar removed the good first issue Good for newcomers label Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants