ocramz / xeno

Fast Haskell XML parser
Other
120 stars 33 forks source link

Parsing fixes #11

Closed qrilka closed 7 years ago

qrilka commented 7 years ago

Fixes #7 #6 While debugging fixes for root node behind spases I was seeing quite suspicious numbers in "DOM indexes" array r so I think it needs some extra tests and maybe some fixes

qrilka commented 7 years ago

@ocramz any news on this one?

chrisdone commented 7 years ago

This is cool, thanks. I'll let @ocramz be the judge. In the spirit of xeno's performance golfing, this is the diff of the benchmarks before and after:

https://gist.github.com/chrisdone/461199db82d06707e0d2ae16bc8cb5ab/revisions

This makes xeno's SAX parser allocate 8x more memory and adds 10%~ extra time. It's possible that this is an inevitable result of fixing things, but there may be something you can shave off.

qrilka commented 7 years ago

Oh, thanks @chrisdone for your feedback, I'll take a look into that higher memory consumption - it looks suspicious. I plan to use xeno for fast "unsafe" parsing in xlsx library and without these fixes it just doesn't work

qrilka commented 7 years ago

@chrisdone it appears that INLINE on parseName which just checked 1st symbol resulted in those extra allocations, removing it brings them back to previous levels - https://gist.github.com/qrilka/71843bc4fb8643b1aa341bcec3f9bc17/revisions

chrisdone commented 7 years ago

@qrilka Looks good to me!

qrilka commented 7 years ago

@ocramz did you have time to look into it?

ocramz commented 7 years ago

Sorry @qrilka for the delay; thanks for your contribution!

qrilka commented 7 years ago

thanks @ocramz Could I hope for a new package release on Hackage soon? I'd like to add it as a dependency toxlsx`

ocramz commented 7 years ago

@qrilka Yep, the latest version with your additions is up now! http://hackage.haskell.org/package/xeno-0.3 : )

qrilka commented 7 years ago

@ocramz sorry for repeating myself but changelog for 0.3 on Hackage doesn't say anything about the version 0.3 it refers too...

ocramz commented 7 years ago

Ooops, I always forget to update changelogs

ocramz commented 7 years ago

@qrilka could you help me here perhaps? how to summarise your addition?

qrilka commented 7 years ago

Sure, it's mainly 2 things:

ocramz commented 7 years ago

Thanks @qrilka , I've updated the changelog in http://hackage.haskell.org/package/xeno-0.3.1