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

Validate item names/ids without allocating Vec #2675

Closed
Erigara opened this issue Aug 30, 2022 · 14 comments
Closed

Validate item names/ids without allocating Vec #2675

Erigara opened this issue Aug 30, 2022 · 14 comments
Labels
good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST Optimization Something isn't working as well as it should

Comments

@Erigara
Copy link
Contributor

Erigara commented Aug 30, 2022

We have a few places where we allocate a vector within the FromStr implementation for an element name/id when we don't really need to.

Approach without allocation: data_model/src/trigger/mod.rs impl FromStr for Id.

@Erigara Erigara added good first issue Good for newcomers Optimization Something isn't working as well as it should labels Aug 30, 2022
@appetrosyan
Copy link
Contributor

https://docs.rs/zerocopy/latest/zerocopy/ Might be useful

@Erigara Erigara added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Aug 30, 2022
@Retssaze
Copy link
Contributor

Where this data_model/src/trigger/mod.rs is located?

@appetrosyan
Copy link
Contributor

Hi! This is an iroha version 2 issue, so you need to switch to the iroha2-dev branch.

@Retssaze
Copy link
Contributor

Retssaze commented Nov 24, 2022 via email

@fofyou
Copy link

fofyou commented Dec 8, 2022

@appetrosyan zerocopy make sense on packat deserialization from network bytes, but how can it do validation on invalid input? for example, no @ in account_id(which should have format like name@domain_name).

after look at data_model/src/trigger/mod.rs, we can do it just replace

let vector: Vec<&str> = string.split('@').collect();

with

let mut split = s.split('@');
match (split.next(), split.next(), split.next()) {

That`s right?

@Erigara
Copy link
Contributor Author

Erigara commented Dec 8, 2022

@suiwenfeng, yes, this was my initial idea.

@fofyou
Copy link

fofyou commented Dec 8, 2022

Ok, i will refactor such code. Method execution time did improved in my local benchmark.

image

@Erigara
Copy link
Contributor Author

Erigara commented Dec 8, 2022

Great to hear! Will be waiting for the PR.

@Retssaze
Copy link
Contributor

Retssaze commented Dec 8, 2022

I would like to do it

@fofyou
Copy link

fofyou commented Dec 9, 2022

#3003

@Retssaze
Copy link
Contributor

Retssaze commented Dec 9, 2022

#3003

You took my task, what a shame!

@6r1d
Copy link
Contributor

6r1d commented Dec 9, 2022

While I am not sure it helps, there's a list of several issues that may be interesting for the new contributors.

@Erigara
Copy link
Contributor Author

Erigara commented Dec 9, 2022

For example you can take a look at #2123.

@Retssaze
Copy link
Contributor

Retssaze commented Dec 9, 2022

To suiwenfeng: Sir, For sure you are very strong and quick, and a message above by 6r1d is definitely not only for you!

appetrosyan pushed a commit that referenced this issue Dec 13, 2022
Signed-off-by: Wenfeng Sui suiwenfeng@qq.com
Signed-off-by: wfsui <wfsui@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST Optimization Something isn't working as well as it should
Projects
Archived in project
Development

No branches or pull requests

5 participants