Skip to content

Timc uv basics#5

Open
timbertson wants to merge 4 commits intoTimWhiting:uv-basicsfrom
timbertson:timc-uv-basics
Open

Timc uv basics#5
timbertson wants to merge 4 commits intoTimWhiting:uv-basicsfrom
timbertson:timc-uv-basics

Conversation

@timbertson
Copy link
Collaborator

The first commit has some timer API changes:

  • removed the APIs not necessary for setTimeout/clearTimeout. JS doesn't support any of these, so they seem unlikely to be used.
  • some possible correctness fixes by ensuring we set callback to null when using/dropping it
  • you can't resume a timer after stopping it. Stopping removes the callback, but you can call start() with new arguments to reuse a timer. This makes callback management simpler, we can remove the callback in stop rather than explicitly having to release the callback.

The second commit is nonfunctional cleanups which I think are also useful, and removes some differences between this branch and my v2 branch:

  • remove a bunch of macros from utils.h which are either unused or only used once. In my v2 branch I have largely replaced these macros with functions for clarity, but we don't need either just yet.
  • more descriptive / consistent naming for some macros/functions
  • replace casting with a couple of macro-generated typed functions (uv_##uv_hnd_tp##_as_kk, kk_##uv_hnd_tp##_box) for some added safety.

Remaining differences

One main difference remains that in my v2 branch, the generated structures all have a common prefix:

  typedef struct kk_##uv_type##_s { \
    kk_uv_flags_t flags; \
    kk_function_t callback; \
    kk_box_t box; \
    uv_type##_t uv; \
  } kk_##uv_type##_t; \

Whereas here, every struct has a callback but its offset within the struct is different per UV type, because the (differently-sized) uv handle is first member of the struct:

  typedef struct kk_##uv_hnd_tp##_s { \
    /* The uv handle struct (embedded as first member) */ \
    uv_##uv_hnd_tp##_t uv; \
    /* 
       The Koka callback function
       Needs to be dupped every time it is called, so that it always can be called again by libuv 
    */ \
    kk_function_t callback; \
  } kk_##uv_hnd_tp##_t; \

This means we can't write generic functions to e.g. take / drop the callback for any kk_uv wrapper struct. So I think we should rearrange these fields, but I haven't done that yet (there's no need with just one struct type, but I think we'll want it later).

@TimWhiting
Copy link
Owner

The first commit has some timer API changes:

  • removed the APIs not necessary for setTimeout/clearTimeout. JS doesn't support any of these, so they seem unlikely to be used.
  • some possible correctness fixes by ensuring we set callback to null when using/dropping it
  • you can't resume a timer after stopping it. Stopping removes the callback, but you can call start() with new arguments to reuse a timer. This makes callback management simpler, we can remove the callback in stop rather than explicitly having to release the callback.

It actually looks like (at least from the libuv perspective) you can issue a timer_stop followed by timer_again, and it will use the callback currently registered. This seems beneficial to me from the perspective of a "pause" feature. Bad naming on the libuv side of things, but it seems like a useful feature to have. As far as implementing it in JS, we would probably have to just stop the timer and start a new one. We could also do the same in Koka for a pause feature (or set a boolean flag not to call a user callback), but it seems to be a waste of cycles in Koka if libuv already supports it, where JS we have do it either way. Maybe this is just me justifying keeping the libuv stuff, but I imagine Koka as a C first language, and JS second, and I feel like people would appreciate whatever is available in C (and be okay with an error in JS if it is not available). We could probably add it later though after requests.

The second commit is nonfunctional cleanups which I think are also useful, and removes some differences between this branch and my v2 branch:

  • remove a bunch of macros from utils.h which are either unused or only used once. In my v2 branch I have largely replaced these macros with functions for clarity, but we don't need either just yet.

I'm fine with this. See the note below about the ordering of fields though.

  • more descriptive / consistent naming for some macros/functions

Sounds good to me.

  • replace casting with a couple of macro-generated typed functions (uv_##uv_hnd_tp##_as_kk, kk_##uv_hnd_tp##_box) for some added safety.

Sounds good.

Remaining differences

One main difference remains that in my v2 branch, the generated structures all have a common prefix:
Whereas here, every struct has a callback but its offset within the struct is different per UV type, because the (differently-sized) uv handle is first member of the struct:
This means we can't write generic functions to e.g. take / drop the callback for any kk_uv wrapper struct. So I think we should rearrange these fields, but I haven't done that yet (there's no need with just one struct type, but I think we'll want it later).

I hadn't thought about the generic functions for dropping callbacks. We could probably still do so if given a reference to the uv struct it is built from (as a macro generated function). Its a tradeoff of how often are you accessing callbacks versus accessing the uv struct -> if you access the struct frequently to call uv apis then it is a cast (no-op) the way I have the fields ordered, which is what I was going for. If the struct often accesses the callback, then maybe a more predictable offset would be useful for CPU branch prediction -> but it is predictable anyways if you are using a lot of the same libuv handles.

@timbertson
Copy link
Collaborator Author

timbertson commented Mar 3, 2026

This seems beneficial to me from the perspective of a "pause" feature.

It exists in UV so obviously there are use cases out there which we'll likely want to support eventually, but I'd prefer to keep that complexity out of the basics branch given that nothing needs it yet. I wouldn't rule out adding it back in later if there's a good use case, but if nothing's using / testing it I also have low confidence it will work correctly.

I hadn't thought about the generic functions for dropping callbacks. We could probably still do so if given a reference to the uv struct it is built from (as a macro generated function).

For clarity here's the generic drop I wrote:

https://github.com/koka-community/uv/blob/5972f73ffbf97b1ee9704180b5651b22f0d5027b/uv/inline/handle.h#L326-L342

kk_uv_any_t is the base structure with all the common fields, and the macro for each type generates a kk_uv_[type]_as_any() casting function which takes in the specific struct and returns the base kk_uv_any_t. This was all inspired by uv_handle_t itself, which has a common set of standard fields at the start so that things like uv_handle_close can accept any handle.

Its a tradeoff of how often are you accessing callbacks versus accessing the uv struct -> if you access the struct frequently to call uv apis then it is a cast (no-op) the way I have the fields ordered, which is what I was going for.

TBH I haven't considered the efficiency here, just developer ergonomics (the less macros & casts I have to deal with the better). I wouldn't have thought access costs would play much of a part in most UV ops though compared to IO and allocations. Happy to defer this decision though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants