elixir-cldr / cldr

Elixir implementation of CLDR/ICU
Other
447 stars 33 forks source link

JSON data read at runtime #127

Closed LostKobrakai closed 5 years ago

LostKobrakai commented 5 years ago

As mentioned here Money.to_string is currently quite slow and seems to do quite a bit of json reading, which it really shouldn't. The offending code seems to be
https://github.com/kipcole9/cldr/blob/master/lib/cldr.ex#L1242, which ultimately lands at Cldr.Config.number_systems. This seems to be done 4 times per Money.to_string in my research.

kipcole9 commented 5 years ago

@LostKobrakai Thanks for looking into this. Fixed with this commit.

Published to hex as version 2.7.1.

Performance difference is materially different as expected:

Version   Name                ips        average  deviation         median         99th %
2.7.0     to_string        334.14        2.99 ms    ±24.62%        2.79 ms        5.41 ms
2.7.1     to_string        8.98 K      111.34 μs    ±29.45%         102 μs         224 μs
kipcole9 commented 5 years ago

As mentioned here Money.to_string is currently quite slow and seems to do quite a bit of json reading, which it really shouldn't. The offending code seems to be https://github.com/kipcole9/cldr/blob/master/lib/cldr.ex#L1242, which ultimately lands at Cldr.Config.number_systems. This seems to be done 4 times per Money.to_string in my research.

@LostKobrakai May I ask what versions of ex_cldr_numbers you are using? I did some work a couple of releases ago to remove the need to validate options (like number_system) multiple times so I'd like to track this down.

kipcole9 commented 5 years ago

Hmmm, I profiled again and I see what you observed (and more) on the latest releases. I'll take a look at this tonight, there's clearly more room for improvement.

LostKobrakai commented 5 years ago

I just did a dummy install (so latest version before todays releases) and used :dbg to trace up the callstack from Jason.decode! by running Money.to_string in iex, because I had no idea what code in cldr would even call it :D