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

Optimize cinn_cache_key by replace GraphToProgram to Dot string #37317

Merged
merged 4 commits into from
Nov 19, 2021

Conversation

thisjiang
Copy link
Contributor

PR types

Performance optimization

PR changes

Others

Describe

优化cinn_cache_key 的生成方式,通过替换非常耗时的GraphToProgram为简约的Dot字符串

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

Verified

This commit was signed with the committer’s verified signature.
suemto suem
… optimize_cache_key
std::string CinnCacheKey::HashGraph(const ir::Graph& graph) {
// using Dot to unqiue graph
inference::analysis::Dot dot;
std::unordered_map<const ir::Node*, std::string> node2dot;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question, can we reuse id of ir::Node instead of creating your own id?

In that case we can save the std::unordered_map data structure and corresponding insert/find operations

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node.h states that DO NOT USE node id. To be discussed in the future.

@@ -39,13 +44,42 @@ CinnCacheKey::CinnCacheKey(const ir::Graph& graph,
this->SetKey(graph, input_shapes, arch_str);
}

std::string CinnCacheKey::HashGraph(const ir::Graph& graph) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hash usually return hash type. Can we call it GraphHashString or GraphHashStr?

Or you just return hash type at line 73 while let caller of this function make it to string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.Changed to return size_t.

Copy link
Member

@zhhsplendid zhhsplendid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@Xreki Xreki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for op benchmark ci

@thisjiang thisjiang merged commit edc3496 into PaddlePaddle:develop Nov 19, 2021
@thisjiang thisjiang deleted the optimize_cache_key branch November 19, 2021 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants