Replace the current file IO solution with one that uses roc buffers #278
Conversation
|
@faldor20 were you able to run the |
| readBytesToBuf!, | ||
| ] | ||
|
|
||
| # import Shared exposing [ByteReader] |
There was a problem hiding this comment.
This should be removed before merging.
| path = Path.fromStr pathStr | ||
| buffer = List.withCapacity 8000 | ||
|
|
||
| # 0 means with default capacity |
There was a problem hiding this comment.
This comment seems to be out of date.
| openReader! : Str => Result Reader [GetFileReadErr Path ReadErr] | ||
| openReader! = \pathStr -> | ||
| path = Path.fromStr pathStr | ||
| buffer = List.withCapacity 8000 |
There was a problem hiding this comment.
Can we move this to a constant a named variable for context on the value of the number?
| fileReader! : List U8, U64 => Result FileReader Str | ||
| fileReadLine! : FileReader => Result (List U8) Str | ||
| fileReader! : List U8 => Result FileReader Str | ||
| fileReadLine! : FileReader,List U8 => Result (List U8) Str |
There was a problem hiding this comment.
Missing a space after the comma. Maybe we should have a formatting CI action? Pretty minor.
| PlatformTasks.fileReadLine! reader buffer | ||
| |> Result.mapErr \err -> FileReadErr path err | ||
|
|
||
| ## Try to read bytes from a file given a Reader. |
There was a problem hiding this comment.
Traditionally, doc comments have a newline after the short description.
This allows the LSP to optionally give a one-line description of the function without including the full description in the paragraph below.
| let size = file | ||
| .metadata() | ||
| .map(|m| m.len()) | ||
| .expect("TODO: make robust: file has not size?"); |
There was a problem hiding this comment.
It seems like getting file metadata can fail when the file is missing or permissions aren't sufficient. Both scenarios seem plausible to me, so we should gracefully handle this error. We should be able to handle the error exactly the same way we handle the other errors in this function.
| let buffer = if buffer.is_unique() { | ||
| buffer | ||
| } else { | ||
| RocList::with_capacity(8000) |
There was a problem hiding this comment.
Also consider contextualizing this magic number.
| loop { | ||
| let read = match file.read(buf_slice) { | ||
| Ok(n) => n, | ||
| Err(ref e) if matches!(e.kind(), ErrorKind::Interrupted) => continue, |
There was a problem hiding this comment.
nitpick: might want to have a retry limit?
3 participants
This replaces the existing buffered IO with a method using only roc buffers.
Massively improves performance in my testing
See this conversation for many benchmarks:
https://roc.zulipchat.com/#narrow/channel/302903-platform-development/topic/Returning.20a.20modified.20roc.20list.20from.20a.20rust.20platform
Some highlights: