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

ENHANCED: Read more characters at once, significantly speeding up library(pio) #589

Merged
merged 3 commits into from
Jun 14, 2020

Conversation

triska
Copy link
Contributor

@triska triska commented Jun 12, 2020

Enjoy!

@ghost
Copy link

ghost commented Jun 12, 2020

Why read one byte at a time? There is take. And for the name string may be matter than str (less confusion with the type str).

@triska
Copy link
Contributor Author

triska commented Jun 12, 2020

Yes, I noticed take, however I found the description unclear in case of errors ("may succeed": what is that supposed to mean, that it may also fail even if the error is resolved?).

In fact I chose str also to avoid confusion with the type String!

@ghost
Copy link

ghost commented Jun 12, 2020

There is the type str used as &str or Box<str> but for string there isn't such type but it's close to String.

The current implementation is like take, on the first error it halts but that doesn't mean EOF has been reached.

triska added a commit to triska/scryer-prolog that referenced this pull request Jun 12, 2020
Suggested by @notoria in mthom#589. Many thanks!
triska added a commit to triska/scryer-prolog that referenced this pull request Jun 12, 2020
@triska
Copy link
Contributor Author

triska commented Jun 12, 2020

Thank you @notoria for these suggestions, I have addressed these issues in additional commits.

Please review, and merge if applicable! Thank you again.

@ghost
Copy link

ghost commented Jun 12, 2020

I don't have the numbers but an iterator could be faster (and nicer?). Something like this, it could be done for both binary and text mode. With push there could be reallocation.

@triska
Copy link
Contributor Author

triska commented Jun 12, 2020

I suggest to leave micro-optimizations to contributors who want to measure the performance of such changes, and ideally address this in Rust itself where possible.

triska added a commit to triska/scryer-prolog that referenced this pull request Jun 13, 2020
Suggested by @notoria in mthom#589. Many thanks!
triska added a commit to triska/scryer-prolog that referenced this pull request Jun 13, 2020
@triska
Copy link
Contributor Author

triska commented Jun 13, 2020

I have rebased the changes against the current master branch!

One additional note: The next and final big step to improve efficiency is to map files without 0-bytes directly into memory using mmap (see #251 ). However, for now, reading in larger chunks as implemented in these commits already improves performance significantly, and also directly uses the compact internal representation for strings.

triska added a commit to triska/scryer-prolog that referenced this pull request Jun 13, 2020
Suggested by @notoria in mthom#589. Many thanks!
triska added a commit to triska/scryer-prolog that referenced this pull request Jun 13, 2020
@mthom mthom merged commit 005220b into mthom:master Jun 14, 2020
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.

2 participants