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

want: add file.readAt() and file.writeAt() #3508

Closed
axetroy opened this issue Dec 15, 2019 · 11 comments
Closed

want: add file.readAt() and file.writeAt() #3508

axetroy opened this issue Dec 15, 2019 · 11 comments
Labels
cli related to cli/ dir public API related to "Deno" namespace in JS suggestion suggestions for new features (yet to be agreed)

Comments

@axetroy
Copy link
Contributor

axetroy commented Dec 15, 2019

Offset operations can currently be performed with seek, but some concise APIs are missing

These APIs are more semantic and easier to understand

Golang also provides a similar API

import (
    "fmt"
    "os"
)

func main() {
    f, _ := os.OpenFile("1.go", os.O_RDWR, os.ModePerm)
    f.WriteAt([]byte("widuu"), 10) // write at offset = 10
    b := make([]byte, 20)
    d, _ := f.ReadAt(b, 10)    // read from offset = 10
    fmt.Println(string(b[:d])) //widuudhellowordhello
}

file.readAt(bytes: Uint8Array, offset: number)

const file = Deno.open("./file.txt")

const bytes = new Uint8Array(10)

file.readAt(bytes, 20); // Read 10 bytes of data with an offset of 20

mock api

async function readAt(bytes: Uint8Array, offset: number) {
  await this.seek(offset, 0)
  await this.read(b)
  await this.seek(0, 0) // reset offset
  return b
}

file.writeAt(bytes: Uint8Array, offset: number)

same as above

Why

Should have the API to quickly read/write some data from file

Or maybe extend the Deno.read(b: Uint8Array) API to Deno.read(b: Uint8Array, offset?: number) will be better?

@kevinkassimo
Copy link
Contributor

kevinkassimo commented Dec 15, 2019

Worth noting glibc posix implements offset-based pread, pwrite simply as seek then write, so IMO we don't really need to extend Deno cli API on Deno.read for it.

NVM, adding to Deno.read as an optional argument looks good to me (taking advantage of atomicity of pread/pwrite)

@caspervonb
Copy link
Contributor

caspervonb commented May 22, 2020

Am in need of this for implementing pread/pwrite in https://deno.land/x/wasi.

Adding an optional offset is the way to go IMHO.

@fvkramer
Copy link

fvkramer commented Jun 9, 2020

This issue has been open for a while - I'll work on it.

@Caryyon
Copy link

Caryyon commented Jun 9, 2020

I'm jumping on this one.

@caspervonb
Copy link
Contributor

Is anyone actively working on this?

@fvkramer
Copy link

fvkramer commented Jul 2, 2020

Hey, I'm not. I assumed the person who posted after me was taking it. It doesn't look like anyone is working on it

@Caryyon
Copy link

Caryyon commented Jul 6, 2020

@caspervonb @fvkramer i have something started but i'm also having a kid like with in 2 weeks.... so if you get this done just toss it out there.

@lucacasonato lucacasonato added cli related to cli/ dir public API related to "Deno" namespace in JS suggestion suggestions for new features (yet to be agreed) labels Sep 14, 2020
@Paosder
Copy link

Paosder commented Dec 17, 2020

Any updates? I'm really looking forward to it.

@lucacasonato
Copy link
Member

You can already do this with Deno.seek

@caspervonb
Copy link
Contributor

It's a worth-while optimisation but I won't be getting around to it for a while.
In the meantime you can fake it with seek.

WASI does that and the performance hit is not that terrible.
https://deno.land/[email protected]/wasi/snapshot_preview1.ts#L795

@crowlKats
Copy link
Member

The goal of built-ins is to have a reasonable set of APIs on which further capabilities can be abstracted on; this feature seems to me more like something that could be done in std, and don't see the necessity to implement this for performance benefits. Especially with the various improvements to the op layer, performance concerns shouldn't be as bad anymore as when this issue was being discussed, so closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir public API related to "Deno" namespace in JS suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

8 participants