mathiasbynens / luamin

A Lua minifier written in JavaScript
https://mths.be/luamin
MIT License
217 stars 54 forks source link

Look for performance improvements #3

Open mathiasbynens opened 11 years ago

mathiasbynens commented 11 years ago

Project goal: focus first on correctness rather than speed.

Once all bugs are fixed, let’s run some benchmarks and take a look at performance improvements.

mathiasbynens commented 11 years ago

Let’s assume the tokens used in ParseLua.lua are representative of an average Lua program (more or less) and order the statement and expression types in luamin based on http://oxyc.github.com/luaparse/examples/stats.html (as seen in @oxyc’s thesis). This way, we optimize for the most commonly used tokens. This could result in performance benefits.

mathiasbynens commented 11 years ago

Also, I think joinStatements can be refactored a bit, so that just a single regular expression is needed for each if/else branch.

oxyc commented 11 years ago

I was curious about how much time the parsing takes in comparsion so I ran it through d8 tick processor. If you haven't done it already here's sample results from minifying ParseLua.lua https://gist.github.com/oxyc/5231186

oxyc commented 11 years ago

For reference, by using https://github.com/oxyc/luaparse/blob/master/lib/luaparse.js#L977:L1004 (note, isKeyword() doesnt check true, false, nil) you'd get ~5% performance increase with v8. Maybe not worth it though

mathiasbynens commented 11 years ago

Good to know! Esprima uses the same trick IIRC. I’ll replace it.

Idea: what do you think about luaparse exposing internal functions like isKeyword for use in other programs?

oxyc commented 11 years ago

Hmm maybe. The fact that true, false, nil are not considered keywords internally while they are according to the spec will probably lead to problems for every first attempt at using the function... This sort of nudges me towards it being too internal to expose. Or what do you think?

mathiasbynens commented 11 years ago

Agreed for this specific case. Still, it could potentially be useful to expose other methods (which) that are 100% spec-compliant. Let’s wait until an actual use case comes up, though ;)