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

efficient event execution on prefix lookup #60

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tsloughter
Copy link
Contributor

I'd been meaning to try this to see what the implementation looked like. Annoying that it requires that recursive function to build the match spec, matchspec should have some sort of operation that lets you specify something is a prefix or EventName ++ ['$tail'] that results in the match being a tail and not a single element... but I digress.

I'd be curious in benchmarks. I'm not really expecting this to be accepted but I thought it better to actually do the comparison instead of assuming we can't do it.

My understanding was that the original attempt at doing prefix matching was still using a bag? Meaning it was scanning the whole table, so particularly slow. In this version I get around needing a bag by appending the handler id to the event name, doesn't change the lookup since it is a prefix lookup.

Or was the original attempt also an ordered_set and still proved to be far too inefficient to be viable?

@binaryseed
Copy link
Contributor

My thought is that I'm hoping that this won't really be needed, we just need to stick to the guidance of static event names + dynamic metadata for further filtering by the handlers.

@arkgil
Copy link
Contributor

arkgil commented Jun 18, 2020

@tsloughter I just found the original implementation and you're right, it used a bag. I agree with @binaryseed, though, I think this ship has sailed and switching to prefixes now will bring confusion as to what is the "right" approach: prefixes or metadata.

Base automatically changed from master to main March 9, 2021 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants