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 the performance of serialize method #3208

Closed
TennyZhuang opened this issue Oct 16, 2021 · 3 comments
Closed

Optimize the performance of serialize method #3208

TennyZhuang opened this issue Oct 16, 2021 · 3 comments
Labels
feature request Suggest an idea on this project welcome contribution

Comments

@TennyZhuang
Copy link
Contributor

TennyZhuang commented Oct 16, 2021

Concisely describe the proposed feature

Currently, we use virtual std::string serialize() = 0; in Expression interface. The serialize which returns a string is concise, but it's inefficient for a tree-style data structure. e.g. If our tree has 10 level and 30 nodes, we have to do at least 30 buffer allocations.

Describe the solution you'd like (if any)

Maybe we can pass a std::stringstream in the serialize, or some other custom appended, e.g.:

void virtual serialize(std::stringstream&);

We can use less allocations to improve the performance, e.g., for GlobalPtrExpression:

void GlobalPtrExpression::serialize(std::stringstream& builder) {
  builder << snode ? snode->get_node_type_name_hinted() : var.serialize();
  builder << '[';
  for (int i = 0; i < (int)indices.size(); i++) {
    indices.exprs[i]->serialize(builder);
    if (i + 1 < (int)indices.size())
      builder << ', ';
  }
  builder << ']';
}

Additional comments

@TennyZhuang TennyZhuang added the feature request Suggest an idea on this project label Oct 16, 2021
@TennyZhuang TennyZhuang changed the title Optimize the performance of serializable method Optimize the performance of serialize method Oct 16, 2021
@skyzh
Copy link
Contributor

skyzh commented Oct 16, 2021

std::stringstream should be std::stringstream&?

@k-ye
Copy link
Member

k-ye commented Oct 18, 2021

Good point! Feel free to send a patch if you'd like to :-)

@strongoier
Copy link
Contributor

Fixed by @TennyZhuang in #3221.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Suggest an idea on this project welcome contribution
Projects
None yet
Development

No branches or pull requests

4 participants