Stream: git-wasmtime

Topic: wasmtime / PR #1329 [wasi-common] add custom FdSet contai...

Wasmtime GitHub notifications bot (Mar 16 2020 at 17:56):

kubkon opened PR #1329 from fd_handles to master:

This PR adds a custom FdSet container which is intended
for use in wasi-common to track WASI fd allocs/deallocs. The
main aim for this container is to abstract away the current
approach of spawning new handles

fd = fd.checked_add(1).ok_or(...)?;

and to make it possible to reuse unused/reclaimed handles
which currently is not done.

The struct offers 3 methods to manage its functionality:

• FdSet::new initialises the internal data structures,
  and most notably, it preallocates an FdSet::BATCH_SIZE
  worth of handles in such a way that we always start popping
  from the "smallest" handle (think of it as of reversed stack,
  I guess; it's not a binary heap since we don't really care
  whether internally the handles are sorted in some way, just that
  the "largets" handle is at the bottom. Why will become clear
  when describing allocate method.)

• FdSet::allocate pops the next available handle if one is available.
  The tricky bit here is that, if we run out of handles, we preallocate
  the next FdSet::BATCH_SIZE worth of handles starting from the
  latest popped handle (i.e., the "largest" handle). This
  works only because we make sure to only ever pop and push already
  existing handles from the back, and push _new_ handles (from the
  preallocation step) from the front. When we ultimately run out
  of _all_ available handles, we then return None for the client
  to handle in some way (e.g., throwing an error such as WasiError::EMFILE
  or whatnot).

• FdSet::deallocate returns the already allocated handle back to
  the pool for further reuse.

When figuring out the internals, I've tried to optimise for both
alloc and dealloc performance, and I believe we've got an amortised
O(1)~* performance for both (if my maths is right, and it may very
well not be, so please verify!).

In order to keep FdSet fairly generic, I've made sure not to hard-code
it for the current type system generated by wig (i.e., wasi::__wasi_fd_t
representing WASI handle), but rather, any type which wants to be managed
by FdSet needs to conform to Fd trait. This trait is quite simple as
it only requires a couple of rudimentary traits (although std:#️⃣:Hash
is quite a powerful assumption here!), and a custom method

Fd::next(&self) -> Option<Self>;

which is there to encapsulate creating another handle from the given one.
In the current state of the code, that'd be simply u32::checked_add(1).
When wiggle makes it way into the wasi-common, I'd imagine it being
similar to

fn next(&self) -> Option<Self> {
    self.0.checked_add(1).map(Self::from)
}

Anyhow, I'd be happy to learn your thoughts about this design!

Wasmtime GitHub notifications bot (Mar 16 2020 at 17:56):

kubkon requested alexcrichton, pchickey, and sunfishcode for a review on PR #1329.

Wasmtime GitHub notifications bot (Mar 16 2020 at 17:56):

kubkon requested alexcrichton, pchickey, and sunfishcode for a review on PR #1329.

Wasmtime GitHub notifications bot (Mar 16 2020 at 17:56):

kubkon requested alexcrichton, pchickey, and sunfishcode for a review on PR #1329.

Wasmtime GitHub notifications bot (Mar 16 2020 at 17:57):

kubkon updated PR #1329 from fd_handles to master:

This PR adds a custom FdSet container which is intended
for use in wasi-common to track WASI fd allocs/deallocs. The
main aim for this container is to abstract away the current
approach of spawning new handles

fd = fd.checked_add(1).ok_or(...)?;

and to make it possible to reuse unused/reclaimed handles
which currently is not done.

The struct offers 3 methods to manage its functionality:

• FdSet::new initialises the internal data structures,
  and most notably, it preallocates an FdSet::BATCH_SIZE
  worth of handles in such a way that we always start popping
  from the "smallest" handle (think of it as of reversed stack,
  I guess; it's not a binary heap since we don't really care
  whether internally the handles are sorted in some way, just that
  the "largets" handle is at the bottom. Why will become clear
  when describing allocate method.)

• FdSet::allocate pops the next available handle if one is available.
  The tricky bit here is that, if we run out of handles, we preallocate
  the next FdSet::BATCH_SIZE worth of handles starting from the
  latest popped handle (i.e., the "largest" handle). This
  works only because we make sure to only ever pop and push already
  existing handles from the back, and push _new_ handles (from the
  preallocation step) from the front. When we ultimately run out
  of _all_ available handles, we then return None for the client
  to handle in some way (e.g., throwing an error such as WasiError::EMFILE
  or whatnot).

• FdSet::deallocate returns the already allocated handle back to
  the pool for further reuse.

When figuring out the internals, I've tried to optimise for both
alloc and dealloc performance, and I believe we've got an amortised
O(1)~* performance for both (if my maths is right, and it may very
well not be, so please verify!).

In order to keep FdSet fairly generic, I've made sure not to hard-code
it for the current type system generated by wig (i.e., wasi::__wasi_fd_t
representing WASI handle), but rather, any type which wants to be managed
by FdSet needs to conform to Fd trait. This trait is quite simple as
it only requires a couple of rudimentary traits (although std:#️⃣:Hash
is quite a powerful assumption here!), and a custom method

Fd::next(&self) -> Option<Self>;

which is there to encapsulate creating another handle from the given one.
In the current state of the code, that'd be simply u32::checked_add(1).
When wiggle makes it way into the wasi-common, I'd imagine it being
similar to

fn next(&self) -> Option<Self> {
    self.0.checked_add(1).map(Self::from)
}

Anyhow, I'd be happy to learn your thoughts about this design!

Wasmtime GitHub notifications bot (Mar 16 2020 at 19:05):

kubkon updated PR #1329 from fd_handles to master:

This PR adds a custom FdSet container which is intended
for use in wasi-common to track WASI fd allocs/deallocs. The
main aim for this container is to abstract away the current
approach of spawning new handles

fd = fd.checked_add(1).ok_or(...)?;

and to make it possible to reuse unused/reclaimed handles
which currently is not done.

The struct offers 3 methods to manage its functionality:

• FdSet::new initialises the internal data structures,
  and most notably, it preallocates an FdSet::BATCH_SIZE
  worth of handles in such a way that we always start popping
  from the "smallest" handle (think of it as of reversed stack,
  I guess; it's not a binary heap since we don't really care
  whether internally the handles are sorted in some way, just that
  the "largets" handle is at the bottom. Why will become clear
  when describing allocate method.)

• FdSet::allocate pops the next available handle if one is available.
  The tricky bit here is that, if we run out of handles, we preallocate
  the next FdSet::BATCH_SIZE worth of handles starting from the
  latest popped handle (i.e., the "largest" handle). This
  works only because we make sure to only ever pop and push already
  existing handles from the back, and push _new_ handles (from the
  preallocation step) from the front. When we ultimately run out
  of _all_ available handles, we then return None for the client
  to handle in some way (e.g., throwing an error such as WasiError::EMFILE
  or whatnot).

• FdSet::deallocate returns the already allocated handle back to
  the pool for further reuse.

When figuring out the internals, I've tried to optimise for both
alloc and dealloc performance, and I believe we've got an amortised
O(1)~* performance for both (if my maths is right, and it may very
well not be, so please verify!).

In order to keep FdSet fairly generic, I've made sure not to hard-code
it for the current type system generated by wig (i.e., wasi::__wasi_fd_t
representing WASI handle), but rather, any type which wants to be managed
by FdSet needs to conform to Fd trait. This trait is quite simple as
it only requires a couple of rudimentary traits (although std:#️⃣:Hash
is quite a powerful assumption here!), and a custom method

Fd::next(&self) -> Option<Self>;

which is there to encapsulate creating another handle from the given one.
In the current state of the code, that'd be simply u32::checked_add(1).
When wiggle makes it way into the wasi-common, I'd imagine it being
similar to

fn next(&self) -> Option<Self> {
    self.0.checked_add(1).map(Self::from)
}

Anyhow, I'd be happy to learn your thoughts about this design!

Wasmtime GitHub notifications bot (Mar 16 2020 at 19:10):

kubkon updated PR #1329 from fd_handles to master:

This PR adds a custom FdSet container which is intended
for use in wasi-common to track WASI fd allocs/deallocs. The
main aim for this container is to abstract away the current
approach of spawning new handles

fd = fd.checked_add(1).ok_or(...)?;

and to make it possible to reuse unused/reclaimed handles
which currently is not done.

The struct offers 3 methods to manage its functionality:

• FdSet::new initialises the internal data structures,
  and most notably, it preallocates an FdSet::BATCH_SIZE
  worth of handles in such a way that we always start popping
  from the "smallest" handle (think of it as of reversed stack,
  I guess; it's not a binary heap since we don't really care
  whether internally the handles are sorted in some way, just that
  the "largets" handle is at the bottom. Why will become clear
  when describing allocate method.)

• FdSet::allocate pops the next available handle if one is available.
  The tricky bit here is that, if we run out of handles, we preallocate
  the next FdSet::BATCH_SIZE worth of handles starting from the
  latest popped handle (i.e., the "largest" handle). This
  works only because we make sure to only ever pop and push already
  existing handles from the back, and push _new_ handles (from the
  preallocation step) from the front. When we ultimately run out
  of _all_ available handles, we then return None for the client
  to handle in some way (e.g., throwing an error such as WasiError::EMFILE
  or whatnot).

• FdSet::deallocate returns the already allocated handle back to
  the pool for further reuse.

When figuring out the internals, I've tried to optimise for both
alloc and dealloc performance, and I believe we've got an amortised
O(1)~* performance for both (if my maths is right, and it may very
well not be, so please verify!).

In order to keep FdSet fairly generic, I've made sure not to hard-code
it for the current type system generated by wig (i.e., wasi::__wasi_fd_t
representing WASI handle), but rather, any type which wants to be managed
by FdSet needs to conform to Fd trait. This trait is quite simple as
it only requires a couple of rudimentary traits (although std:#️⃣:Hash
is quite a powerful assumption here!), and a custom method

Fd::next(&self) -> Option<Self>;

which is there to encapsulate creating another handle from the given one.
In the current state of the code, that'd be simply u32::checked_add(1).
When wiggle makes it way into the wasi-common, I'd imagine it being
similar to

fn next(&self) -> Option<Self> {
    self.0.checked_add(1).map(Self::from)
}

Anyhow, I'd be happy to learn your thoughts about this design!

Wasmtime GitHub notifications bot (Mar 16 2020 at 19:44):

kubkon updated PR #1329 from fd_handles to master:

This PR adds a custom FdSet container which is intended
for use in wasi-common to track WASI fd allocs/deallocs. The
main aim for this container is to abstract away the current
approach of spawning new handles

fd = fd.checked_add(1).ok_or(...)?;

and to make it possible to reuse unused/reclaimed handles
which currently is not done.

The struct offers 3 methods to manage its functionality:

• FdSet::new initialises the internal data structures,
  and most notably, it preallocates an FdSet::BATCH_SIZE
  worth of handles in such a way that we always start popping
  from the "smallest" handle (think of it as of reversed stack,
  I guess; it's not a binary heap since we don't really care
  whether internally the handles are sorted in some way, just that
  the "largets" handle is at the bottom. Why will become clear
  when describing allocate method.)

• FdSet::allocate pops the next available handle if one is available.
  The tricky bit here is that, if we run out of handles, we preallocate
  the next FdSet::BATCH_SIZE worth of handles starting from the
  latest popped handle (i.e., the "largest" handle). This
  works only because we make sure to only ever pop and push already
  existing handles from the back, and push _new_ handles (from the
  preallocation step) from the front. When we ultimately run out
  of _all_ available handles, we then return None for the client
  to handle in some way (e.g., throwing an error such as WasiError::EMFILE
  or whatnot).

• FdSet::deallocate returns the already allocated handle back to
  the pool for further reuse.

When figuring out the internals, I've tried to optimise for both
alloc and dealloc performance, and I believe we've got an amortised
O(1)~* performance for both (if my maths is right, and it may very
well not be, so please verify!).

In order to keep FdSet fairly generic, I've made sure not to hard-code
it for the current type system generated by wig (i.e., wasi::__wasi_fd_t
representing WASI handle), but rather, any type which wants to be managed
by FdSet needs to conform to Fd trait. This trait is quite simple as
it only requires a couple of rudimentary traits (although std:#️⃣:Hash
is quite a powerful assumption here!), and a custom method

Fd::next(&self) -> Option<Self>;

which is there to encapsulate creating another handle from the given one.
In the current state of the code, that'd be simply u32::checked_add(1).
When wiggle makes it way into the wasi-common, I'd imagine it being
similar to

fn next(&self) -> Option<Self> {
    self.0.checked_add(1).map(Self::from)
}

Anyhow, I'd be happy to learn your thoughts about this design!

Wasmtime GitHub notifications bot (Mar 16 2020 at 21:53):

kubkon updated PR #1329 from fd_handles to master:

This PR adds a custom FdSet container which is intended
for use in wasi-common to track WASI fd allocs/deallocs. The
main aim for this container is to abstract away the current
approach of spawning new handles

fd = fd.checked_add(1).ok_or(...)?;

and to make it possible to reuse unused/reclaimed handles
which currently is not done.

The struct offers 3 methods to manage its functionality:

• FdSet::new initialises the internal data structures,
  and most notably, it preallocates an FdSet::BATCH_SIZE
  worth of handles in such a way that we always start popping
  from the "smallest" handle (think of it as of reversed stack,
  I guess; it's not a binary heap since we don't really care
  whether internally the handles are sorted in some way, just that
  the "largets" handle is at the bottom. Why will become clear
  when describing allocate method.)

• FdSet::allocate pops the next available handle if one is available.
  The tricky bit here is that, if we run out of handles, we preallocate
  the next FdSet::BATCH_SIZE worth of handles starting from the
  latest popped handle (i.e., the "largest" handle). This
  works only because we make sure to only ever pop and push already
  existing handles from the back, and push _new_ handles (from the
  preallocation step) from the front. When we ultimately run out
  of _all_ available handles, we then return None for the client
  to handle in some way (e.g., throwing an error such as WasiError::EMFILE
  or whatnot).

• FdSet::deallocate returns the already allocated handle back to
  the pool for further reuse.

When figuring out the internals, I've tried to optimise for both
alloc and dealloc performance, and I believe we've got an amortised
O(1)~* performance for both (if my maths is right, and it may very
well not be, so please verify!).

In order to keep FdSet fairly generic, I've made sure not to hard-code
it for the current type system generated by wig (i.e., wasi::__wasi_fd_t
representing WASI handle), but rather, any type which wants to be managed
by FdSet needs to conform to Fd trait. This trait is quite simple as
it only requires a couple of rudimentary traits (although std:#️⃣:Hash
is quite a powerful assumption here!), and a custom method

Fd::next(&self) -> Option<Self>;

which is there to encapsulate creating another handle from the given one.
In the current state of the code, that'd be simply u32::checked_add(1).
When wiggle makes it way into the wasi-common, I'd imagine it being
similar to

fn next(&self) -> Option<Self> {
    self.0.checked_add(1).map(Self::from)
}

Anyhow, I'd be happy to learn your thoughts about this design!

Wasmtime GitHub notifications bot (Mar 17 2020 at 09:08):

kubkon updated PR #1329 from fd_handles to master:

This PR adds a custom FdSet container which is intended
for use in wasi-common to track WASI fd allocs/deallocs. The
main aim for this container is to abstract away the current
approach of spawning new handles

fd = fd.checked_add(1).ok_or(...)?;

and to make it possible to reuse unused/reclaimed handles
which currently is not done.

The struct offers 3 methods to manage its functionality:

• FdSet::new initialises the internal data structures,
  and most notably, it preallocates an FdSet::BATCH_SIZE
  worth of handles in such a way that we always start popping
  from the "smallest" handle (think of it as of reversed stack,
  I guess; it's not a binary heap since we don't really care
  whether internally the handles are sorted in some way, just that
  the "largets" handle is at the bottom. Why will become clear
  when describing allocate method.)

• FdSet::allocate pops the next available handle if one is available.
  The tricky bit here is that, if we run out of handles, we preallocate
  the next FdSet::BATCH_SIZE worth of handles starting from the
  latest popped handle (i.e., the "largest" handle). This
  works only because we make sure to only ever pop and push already
  existing handles from the back, and push _new_ handles (from the
  preallocation step) from the front. When we ultimately run out
  of _all_ available handles, we then return None for the client
  to handle in some way (e.g., throwing an error such as WasiError::EMFILE
  or whatnot).

• FdSet::deallocate returns the already allocated handle back to
  the pool for further reuse.

When figuring out the internals, I've tried to optimise for both
alloc and dealloc performance, and I believe we've got an amortised
O(1)~* performance for both (if my maths is right, and it may very
well not be, so please verify!).

In order to keep FdSet fairly generic, I've made sure not to hard-code
it for the current type system generated by wig (i.e., wasi::__wasi_fd_t
representing WASI handle), but rather, any type which wants to be managed
by FdSet needs to conform to Fd trait. This trait is quite simple as
it only requires a couple of rudimentary traits (although std:#️⃣:Hash
is quite a powerful assumption here!), and a custom method

Fd::next(&self) -> Option<Self>;

which is there to encapsulate creating another handle from the given one.
In the current state of the code, that'd be simply u32::checked_add(1).
When wiggle makes it way into the wasi-common, I'd imagine it being
similar to

fn next(&self) -> Option<Self> {
    self.0.checked_add(1).map(Self::from)
}

Anyhow, I'd be happy to learn your thoughts about this design!

Last updated: Nov 22 2024 at 17:03 UTC