-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
print/vmc.c: Reduce generated code verbosity for .{1000,}
.
#446
base: main
Are you sure you want to change the base?
Conversation
There was already an existing special case for repeated `FETCH, STOP, FETCH, STOP, ...` sequences, so add a similar case for repeated `FETCH, BRANCH, ...` cases as would be produced by `.{1000,}`.
/* Instead of emitting e.g.: | ||
* if (c = (unsigned char) *p++, c == '\0') return -1; | ||
* if (c == '\n') goto l0; | ||
* N times, emit a for loop around that once. |
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.
okay, this makes a lot of sense!
I wonder if this would be useful for the |
@dgryski I think we should move this logic up a level, to the vm, and introduce an opcode for this, or replace the existing "match a character" with "match a substring". then everybody benefits |
If you look, there are/were plans for opcodes for this. It should speed up certain kinds of matches, but I've never been able to get a test set that would tell me when it helped and when it didn't, so I haven't moved forward with it. It's pretty easy to come up with unanchored REs where this wouldn't help much without some serious graph preprocessing, and where a naive approach may be detrimental. There's also some locality trade-offs between match-a-substring and match-a-character that weren't obvious to me. I've been noodling around with more general graph optimizations over the last few years, but work has been demanding, so I haven't made much progress lately. Sadly, I don't see that changing in the near future. Which is to say: I think this is an interesting direction to explore, and I'm glad you guys have the data to test whether it has an impact for your problems. I would make this an optional optimization in case there are other use cases where this is detrimental to performance. As a last thought, I don't think you want to replace "match a character" with "match a substring." I think you want both to let the failure handling be different. When a substring match fails, you'd want to do something intelligent with where it failed rather than abort the whole substring. @silentbicycle had suggested this at one point, and it's a good idea. |
I'm revisiting this now, and agree that having separate character and substring match instructions makes sense (rather than special-casing it in code generation like this). Conditionally emitting substring match would limit the impact of the change to specific cases like |
In the specific cases I have in mind (
but that wouldn't extend well to even slightly more complicated What instead do you think about either:
The count could be the bottom 16 or so bits, the upper bits would be an address/table-id for a OR
The first instruction would initialize a new monotonic counter to a max count value. The second would decrement the counter value, then fall through to the next instruction if zero, otherwise jump back to immediately after where the counter was initialized for another repetition. Each id would correspond to a local variable in the code generation output -- at an IR level we could treat this like an unlimited register machine. The output language would either figure out the scoping or could use a stack. In practice both the number of IDs and their scope should be quite small. Nothing else should be able to modify the counter in any way, to avoid unbounded looping. This would only be used to handle highly repetitive sequences, and any other instructions jumping into the middle of that instruction sequence from outside should be rejected at compile-time, to keep the code using the counter from being abused somehow that the unrolled version couldn't be. A looped regex whose body can match nothing (like It would also nest, in a way that What do you think? (edit: formatting) |
Note that Ragel https://www.colm.net/open-source/ragel/ has some extensions that allow it to create not-strictly-regular languages involving counters (such as nested parentheses). It might be interesting to see how it solves a similar problem. |
We talked about this, we prefer a single instruction. because it makes the IR rewriting simpler |
There was already an existing special case for repeated
FETCH, STOP, FETCH, STOP, ...
sequences, so add a similar case for repeatedFETCH, BRANCH, ...
cases as would be produced by.{1000,}
.