PiotrMachowski / Python-package-vacuum-map-parser-roborock

Apache License 2.0
1 stars 2 forks source link

feat: optimize getting colors #7

Closed Lash-L closed 6 months ago

Lash-L commented 8 months ago

Depends on https://github.com/PiotrMachowski/Python-package-vacuum-map-parser-base/pull/6

before: image

after: image

Running 10 times each time recreating the parser on a smallish map on my local machine.

I think I could get some pretty significant time improvements by using numba, but i wasn't sure if that is something you would want and I'm not sure if it is worth the time of getting everything to be numba compatable in the parse function.

PiotrMachowski commented 7 months ago

Have you checked which part of changed code impacts the performance? Is it skipping if in get_color, or operating directly on the dict instead of using method from ColorsPalette?

Lash-L commented 7 months ago

Have you checked which part of changed code impacts the performance? Is it skipping if in get_color, or operating directly on the dict instead of using method from ColorsPalette?

So it is just because we don't have to do so much in the function.

get_colors was getting called a lot of times, I wish i still had the profile data, but i don't. But now instead of having to do any math or if statements, we can just grab it from the cache. It isn't that anything was slow per say, it was just that it was happening a lot of times.

PiotrMachowski commented 7 months ago

To be honest I'm not a great fan of this change, because it "leaks" the functionality from ColorsPalette to other places, but I can't argue with numbers and performance increase, so I'll merge it

Lash-L commented 6 months ago

To be honest I'm not a great fan of this change, because it "leaks" the functionality from ColorsPalette to other places, but I can't argue with numbers and performance increase, so I'll merge it

If you have any better ideas how to achieve something similar without leaking, I'm more than happy to take a stab at it!

PiotrMachowski commented 6 months ago

Thank you again @Lash-L, I have finally figured it out 👍