WebAssembly / WebAssembly/wasi-libc

fd_set implementation is inefficient

Open
#283 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
1k
Forks
251
Avg merge
7h 15m
Merged PRs (30d)
3

Description

cloudlibc seems to have made a design choice that POSIX does not require, and what most *NIX implementations don't support. this has led to cloudlibc using a lot more memory supporting scenarios with select() that is better served by poll(). imo this should be revisited by basically deleting the cloudlibc logic entirely.

first, let's start with what POSIX says:

  • fd_set type

    The <sys/select.h> header shall define the fd_set type as a structure.
    The <sys/select.h> header shall define the following symbolic constant, which shall have a value suitable for use in #if preprocessing directives:
    FD_SETSIZE Maximum number of file descriptors in an fd_set structure.
    The requirement for the fd_set structure to have a member fds_bits has been removed as per The Open Group Base Resolution bwg2001-005.

  • select()

    The behavior of these macros is undefined if the fd argument is less than 0 or greater than or equal to FD_SETSIZE, or if fd is not a valid file descriptor, or if any of the arguments are expressions with side-effects.

let's look at what cloudlibc does:

  • FD_SETSIZE set to 1024
  • fd_set type
    • int __fds[FD_SETSIZE]; -- an array of integers (fds)
  • FD_* macros
    • the index into __fds is just "the next used fd"
    • the value in __fds is the fd to check
  • end result:
    • scanning the __fds array takes exactly __nfds checks (good)
    • sizeof(fd_set) is 4100 bytes (bad)
    • still has a max nfds of FD_SETSIZE (fine)
    • any valid fd can be monitored with select(), not just [0, FD_SETSIZE) (good)

let's look at what musl (and basically every other *NIX library out there) does:

  • sys/select.h
    • FD_SETSIZE set to 1024
    • unsigned long fds_bits[FD_SETSIZE / 8 / sizeof(long)]; -- a bitmask of fds
    • the index into fds_bits is the fd to check
    • the value is a single bit and whether to check the fd
  • end result:
    • scanning fds_bits takes longer, but most architectures have "bit is set" insns to optimize this. an unoptimized implementation would take at least 128 tests (FD_SETSIZE / 8) (meh)
    • sizeof(fd_set) is 128 bytes (good)
    • still has a max nfds of FD_SETSIZE (fine)
    • only valid fd's within [0, FD_SETSIZE) can be monitored with select() (fine)

so cloudlibc uses 4k of memory instead of 128bytes in order to be slightly faster and to support fds larger than FD_SETSIZE. but no POSIX-compliant project would ever use an fd >= FD_SETSIZE, which means they'd be making assumptions that the underlying code is cloudlibc.

WASI API isn't relevant because it doesn't define fd_set, and this is purely about the select() API which is entirely part of WASI libc.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the select-related headers referenced in the issue: __macro_FD_SETSIZE.h, __typedef_fd_set.h, _fd_set.h, and musl's sys/select.h. Compare the current fd_set representation with the POSIX constraints and determine the required scope and compatibility impact. Done should include an agreed implementation direction and corresponding validation for select and FD* behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, wasm
Domain
operating-systems
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.