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

design question: lexer in goroutine? #19

Open
julienschmidt opened this issue Apr 25, 2019 · 4 comments
Open

design question: lexer in goroutine? #19

julienschmidt opened this issue Apr 25, 2019 · 4 comments
Assignees

Comments

@julienschmidt
Copy link
Contributor

julienschmidt commented Apr 25, 2019

While looking through the code, I noticed that the lexer runs its main loop its own goroutine:

func lex(input string) (*lexer, error) {
	...
	l := &lexer{
		input: input,
		items: make(chan item),
	}
	go l.run()
	return l, nil
}

Items are then received from the unbuffered channel l.items , thus making the code run fully sequential again.

Is there any other good reason to run it in it's own goroutine? If not, the overhead for synchronization and context switches could be avoided.

@duanehoward
Copy link
Collaborator

@clem1 Any thoughts here. I don't see a strong reason for this to be its own goroutine, but maybe you had given this some thought?

@clem1
Copy link
Collaborator

clem1 commented May 15, 2019

So this code was copied from the official Golang text lexer [0]. I guess the main reason is to not wait for the end of the lexing in the parsing, e.g. parser can exit earlier without having the lexing to be completely finished. Not sure it maters in our case since rules are relatively small and very fast to lex.

Happy to have the lex() being fully sequential.

[0] https://talks.golang.org/2011/lex.slide#1

@duanehoward
Copy link
Collaborator

@julienschmidt do you want to take a look at making this change? I don't know that it gains us a lot to remove it, so I don't plan to tackle this in the short term (versus adding features, better parsing, etc.)
If not I'll close this issue for now.

@julienschmidt
Copy link
Contributor Author

I'm actually curious if this would pay off. I'll write a PoC and benchmark as soon as I have time for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants