serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Everywhere: Some Core::File -> Core::Stream::File conversions

Open AtkinsSJ opened this issue 3 years ago • 1 comments

These are some simpler cases, because I was in the mood for simple.

AtkinsSJ avatar Sep 12 '22 20:09 AtkinsSJ

A few of these are reading into a byte buffer a-la ByteBuffer::create_uninitialized, but of a static size. Can we use e.g. Array<u8, 4096> instead to avoid extra allocation? See: https://github.com/SerenityOS/serenity/blob/master/Meta/Lagom/Tools/CodeGenerators/LibUnicode/GeneratorUtil.h#L337 for an example

trflynn89 avatar Sep 13 '22 14:09 trflynn89

A few of these are reading into a byte buffer a-la ByteBuffer::create_uninitialized, but of a static size. Can we use e.g. Array<u8, 4096> instead to avoid extra allocation? See: master/Meta/Lagom/Tools/CodeGenerators/LibUnicode/GeneratorUtil.h#L337 for an example

Hmmm. That doesn't handle OOM for allocating the array though, right?

AtkinsSJ avatar Sep 14 '22 10:09 AtkinsSJ

Hmmm. That doesn't handle OOM for allocating the array though, right?

Arrays aren't heap allocated

trflynn89 avatar Sep 14 '22 11:09 trflynn89

Ohhh, so we don't need to worry about OOM for stack allocation? That makes sense.

AtkinsSJ avatar Sep 14 '22 15:09 AtkinsSJ

I've replaced use of ByteBuffer::create_uninitialized() with an Array<>. Also, using PAGE_SIZE instead of 4096 as suggested in another PR.

AtkinsSJ avatar Sep 20 '22 10:09 AtkinsSJ