Skip to content
This repository has been archived by the owner on Apr 4, 2021. It is now read-only.

Exposing the lexer/parser #13

Closed
rski opened this issue Sep 25, 2017 · 6 comments
Closed

Exposing the lexer/parser #13

rski opened this issue Sep 25, 2017 · 6 comments

Comments

@rski
Copy link
Contributor

rski commented Sep 25, 2017

Hi,

There is a bit of a backstory to this issue, but basically I'm looking to write my own XPath evaluator, mostly because the one in Goxpath isn't geared towards what I want and that translates in big performance cost. Goxpath does have a great lexer/parser, so there is no point in me reinventing the wheel there.

Would you be open to a PR that moves the parser/lexer out of internal so that I can use them in another package?

@ChrisTrenkamp
Copy link
Owner

I can see why one would want access to the lexer, but you sure you want access to the parser? The parse tree is built in a very particular way so the evaluator can do its job, and if goxpath's evaluator isn't behaving the way you want, building another evaluator around goxpath's parse tree probably won't do you any good.

Also, can you explain where you're seeing some performance issues in goxpath?

@rski
Copy link
Contributor Author

rski commented Sep 26, 2017

The performance issues are because I don't evaluate it on XML exactly, so most of the functions in findUtils are really expensive. On the other hand, I can take some shortcuts that aren't available in XML.

The parse tree is built in a very particular way so the evaluator can do its job, and if goxpath's evaluator isn't behaving the way you want, building another evaluator around goxpath's parse tree probably won't do you any good.

Until now I haven't faced anything like this, benchmarks are looking quite good. It sounds like there is something specific you have in mind wrt any design decisions in the evaluator though, which I might not have hit yet.

@ChrisTrenkamp
Copy link
Owner

ChrisTrenkamp commented Sep 26, 2017

Can you point to which parts in the lexer/parser packages you need? It's probably not as simple as just getting rid of the internal part; a lot of methods and fields are private because they weren't really designed with a public API in mind.

@rski
Copy link
Contributor Author

rski commented Sep 27, 2017

For now, I'm using pretty much everything the current evaluator is using, only in a different way. You can have a look at the changes i've made at https://github.com/aristanetworks/goxpath/commits/master

@ChrisTrenkamp
Copy link
Owner

Your changes look good. Pulling them down.

@rski
Copy link
Contributor Author

rski commented Sep 28, 2017

excellent, thanks a lot!

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

No branches or pull requests

2 participants