-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
Hello, we (@getsentry) have recently started profiling one of our services which runs echarts to generate thumbnail images of charts so that they can be easily shared as previews and while checking some profiles generated in prod, we noticed that a function called each is often at the top of the stack.
Looking at HashMap.each, there could be two possible performance culprits in there.
First is v8 disabling optimization of for...in described here, this could occur in the event that our {} dict exceeds the number of keys and the underlying v8 data structure backing the object starts using an actual hash map which makes enumeration slow.
Second, I noticed that map.get does a call to hasOwnProperty before returning the value, this seems much slower than just returning the value of dict[key], see micro benchmark.
I could see this being improved by using native maps as the underlying class and relying on Map.entries for iteration (there is a performance benefit here in the sense that we can also avoid a second function call to get) as well as possibly removing the hasOwnProperty call.
I would like to get your input on this issue, more specifically, I'd like to better understand the motivation behind not using native maps (assuming this could have been due to bad browsers support at the time?). Full disclaimer that I am not very familiar with the rest of the echarts codebase, so I lack context if this is a very commonly used method or if it just relates to the specific charts we generate. I would also be more than happy to contribute myself if we can agree on a direction.
Appreciate any thoughts and it goes without saying, but thank you for maintaining the library that we use daily 👏🏼
