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

Update the Rust's brainfuck implementation #325

Merged
merged 3 commits into from
Apr 18, 2021

Conversation

ricvelozo
Copy link
Contributor

More idiomatic code, and formatting with rustfmt. The most important change was in the Tape impl:

  • mov() reallocs only once per call;
  • get() and inc() uses unchecked indexing because the check is already done in mov()

More idiomatic code, and formatting with `rustfmt`. The most important change was in the `Tape` impl:

* `mov()` reallocs only once per call;
* `get()` and `inc()` uses unchecked indexing because the check is already done in `mov()`
@ricvelozo
Copy link
Contributor Author

On my machine the improvement was ~2.6 seconds with mandel.b.

@ricvelozo ricvelozo marked this pull request as ready for review April 17, 2021 02:17
@ricvelozo
Copy link
Contributor Author

Can I use a an external crate in this implementation? I want to use an arena allocator to alloc the Loop ops. all at once.

brainfuck/bf.rs Outdated Show resolved Hide resolved
@kostya
Copy link
Owner

kostya commented Apr 17, 2021

Can I use a an external crate in this implementation? I want to use an arena allocator to alloc the Loop ops. all at once.

I not think this is needed, because parse is 0.01% time of benchmark, main heavy operation is run.

Because of the strong typing, the `mov()` was doing many lossy
casts which the C and C++ versions don't; the D version uses
specific variants too.

Besides, the value was always -1 or 1, so it is more idiomatic
this way.
@ricvelozo
Copy link
Contributor Author

On the last commit the improvement was ~2.4 seconds with mandel.b.

@nuald
Copy link
Collaborator

nuald commented Apr 18, 2021

Seems good to me. "unsafe" usage is totally fine here as it's not like the code is somehow unsafe, but rather Rust creators decides to force developers to think twice before accessing elements directly without bounds check. It's not the first time I see the necessity of using "unsafe" constructions in a totally safe code (like in a low-level bits manipulation code).

@nuald nuald merged commit 7e2fc03 into kostya:master Apr 18, 2021
nuald added a commit to nuald/benchmarks that referenced this pull request Apr 18, 2021
nuald added a commit that referenced this pull request Apr 18, 2021
@ricvelozo ricvelozo deleted the brainfuck-rs branch April 18, 2021 17:06
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.

3 participants