Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Speed] scope.Findvar maybe slow down all the operators #9232

Closed
typhoonzero opened this issue Mar 20, 2018 · 4 comments
Closed

[Speed] scope.Findvar maybe slow down all the operators #9232

typhoonzero opened this issue Mar 20, 2018 · 4 comments

Comments

@typhoonzero
Copy link
Contributor

typhoonzero commented Mar 20, 2018

Here's some reference I've searched:
https://softwareengineering.stackexchange.com/questions/49550/which-hashing-algorithm-is-best-for-uniqueness-and-speed

Every operator need to call Findvar directly or indirectly.

As C++ default hash<std::string> implementation uses "murmurhash", which is the fastest hash function, it may still slower than directly lookup a variable using an offset. We can consider a way to make runtime Findvar implementation do not use unordered_map.

@helinwang
Copy link
Contributor

helinwang commented Mar 21, 2018

Thanks for the issue!
Maybe the hash table lookup cost is marginal, comparing to the time it take for running GPU kernels? I think we need to have some profiling numbers to back this up.

@tonyyang-svail
Copy link

@typhoonzero Any follow-ups on this issue? The hash lookup usually takes 200ns. The gap between Op.Run() is usually 500,000 ~ 1,000,000 ns. I doubt the loop up time is the issue.

@dzhwinter
Copy link
Contributor

One month ago, I had discussed this issue with @typhoonzero, we thought that it will not degrade the performance magnitude. Because 200ns means 200 * 10-12, it is really small.
But to give a solid conclusion, now I'm doing an experiment on this changing the unordered_map to index based vector, will report the result later.

@typhoonzero
Copy link
Contributor Author

Closing this for now!

Performance Tuning automation moved this from TODO to Done May 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment