event-ruler icon indicating copy to clipboard operation
event-ruler copied to clipboard

Possibly remove ShortcutTransition

Open timbray opened this issue 3 years ago • 3 comments

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 avatar Aug 03 '22 18:08 timbray

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.

baldawar avatar Aug 09 '22 15:08 baldawar

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

timbray avatar Aug 09 '22 16:08 timbray

Fair point. Let me take that out.

baldawar avatar Aug 09 '22 16:08 baldawar