sirkon / ldetool

Code generator for fast log file parsers
MIT License
317 stars 22 forks source link

Variable constant collision when generating multiple similar extractors #47

Closed sternd closed 2 years ago

sternd commented 2 years ago

Using the ldetool to parse game logs. There are ~30 different types of entries that can be in the log file, and of that I plan to parse ~1/2 of them. Ran into an issue when using strings instead of bytes where I have this defined in multiple files:

var constSpaceMinusSpace = []byte(" - ")

Is there a way to generate the code with these constants being locally scoped to the Extract function, pulled out into its own file of constants, or should I try and convert everything into bytes instead of using strings?

sirkon commented 2 years ago

Is there a way to generate the code with these constants being locally scoped to the Extract function.

No, there's no way. I am afraid putting constSpaceMinusSpace into the Extract scope would mean allocation on each call of it. Log processing hates regular allocations, they should be avoided fanatically.

I guess I can add rule name as a prefix to these vars names to ensure uniqueness. Something like constRuleNameSpaceMinusSpace.

should I try and convert everything into bytes instead of using strings?

Using bytes over strings is highly recommended when it is possible. You have no other choice though if your T phase (of ETL) does some sort of data aggregation.

sirkon commented 2 years ago

Added rule name into constants, please check it out. v0.4.15

sternd commented 2 years ago

Thanks, this will be a perfect fix for it. You're right, allocating memory on each call was a terrible idea for performance. I'll check out v0.4.15 and update on the fix.

sternd commented 2 years ago

Just checked and I was already running v0.4.15 and don't see the rule name in the constants. Is there a different version with this fix in place?

sirkon commented 2 years ago

My bad, haven't pushed changes and have a mess in tags. v0.4.16.

sternd commented 2 years ago

Validated v0.4.16. It works great! Namespaced constants resolved all issues with collisions.