Improve compilation times of gluon_parser

[brendanzab]

This is causing timeout woes with travis and makes things hard for users on stable. As discussed on https://github.com/Marwes/gluon/issues/33#issuecomment-233157725 and https://github.com/Marwes/gluon/pull/66#issuecomment-234803447 we could possibly consider switching to a parser generator like rustpeg of LALRPOP.

[brendanzab]

Assigning myself to this.

[Marwes]

From #66

rustpeg doesn't require nightly if you use a build script ;)

https://github.com/kevinmehall/rust-peg#as-a-standalone-code-generator

Run peg input_file.rustpeg to compile a grammar and generate Rust code on stdout. This code works with stable Rust.

Sounds to me that using rustpeg on stable still requires a built rustpeg executable (which requires nightly to build, but I might be wrong?)

[Marwes]

lalrpop seems to support a custom lexer though it seems to lack documentation for it at the moment.
https://github.com/nikomatsakis/lalrpop/issues/39

Could not find anything about custom lexers for rust-peg

[Marwes]

Looks like lalrpop also has some problems with compile times though :/ https://github.com/nikomatsakis/lalrpop/issues/65

[brendanzab]

no, rustpeg doesn't do lexers.

Shame about LALRPOP... :(

There is this: https://github.com/nikomatsakis/lalrpop/issues/65#issuecomment-198108147

[nikomatsakis]

I just landed a change to LALRPOP that may make a big difference -- you might want to try the trunk and see if things are improved.

[nikomatsakis]

Another related factor is that, due to rust-lang/cargo#2424, LALRPOP runs in debug mode if you are doing a debug build, which makes it quite slow. :( I've been meaning to address this by allowing users to cargo install lalrpop, which will install an optimized binary executable, and then detecting the presence of that executable and shelling out to it instead, but I haven't gotten around to it.

However, this should only matter when you make changes to the grammar -- after that, the .rs file should not be regenerated.

[Marwes]

@nikomatsakis That is some impressive speedup! From https://github.com/nikomatsakis/lalrpop/issues/65#issuecomment-198108147 I take it that it is already possible to use a precompiled release executable so I am assuming you just mean that it should be handled in a more automatic fashion?

Relying on not modifying the parser is pretty much how we get by at the moment so that part will not be any worse at least

[nikomatsakis]

I take it that it is already possible to use a precompiled release executable so I am assuming you just mean that it should be handled in a more automatic fashion?

No, that's a separate thing. I mean, it is actually possible -- but those results I charted there were just obtained by copy-and-pasting the rustc line, so as to exclude the time to generate the .rs file. (Using a precompiled executable would only affect the time to generate the .rs file.)

[nikomatsakis]

To be clear: you can install a LALRPOP executable, but you must write a build.rs script yourself to run it. It's not the recommended way of doing things because it is not automatible by cargo.

[nikomatsakis]

Oh, and -- yes, I guess that comment is precisely what I meant. =)

(I hadn't realized which comment you were referring to...)

[brendanzab]

We could just target a ref of your commit @nikomatsakis? Then experiment on a branch? No need to merge it just yet.

[nikomatsakis]

The branch has been merged to trunk in any case.
On Aug 3, 2016 8:41 PM, Brendan Zabarauskas notifications@github.com wrote:We could just target a ref of your commit @nikomatsakis? Then experiment on a branch? No need to merge it just yet.

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.

[brendanzab]

Oho, great! Any idea when it'll be released to cargo? No hurry - plenty of time for us to experiment still...

[nikomatsakis]

@brendanzab pretty soon, I imagine. I was just hoping to give a bit of time for any bug reports to roll in...