Skip to content

Cache suspensions #37

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

Merged
merged 6 commits into from
Jan 14, 2022
Merged

Cache suspensions #37

merged 6 commits into from
Jan 14, 2022

Conversation

kelunik
Copy link
Member

@kelunik kelunik commented Jan 10, 2022

Not sure whether this is useful. If we go that route, we should rename createSuspension into getSuspension.

Copy link
Member

@trowski trowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

createSuspension I think is a fine name. Decorating drivers may still return a new object implementing Suspension to implement some fiber-locals, etc.

@kelunik
Copy link
Member Author

kelunik commented Jan 10, 2022

create sounds like it always returns a new object, which it doesn't do anymore then.

@trowski
Copy link
Member

trowski commented Jan 11, 2022

create sounds like it always returns a new object, which it doesn't do anymore then.

Yes, I suppose you're right. My initial thinking was create would better indicate the method should be called rather than reusing a prior Suspension object by the consumer (which may have been associated with a different fiber). However, create certainly doesn't make this obvious and will need to be documented anyway. get likely does make more sense then. I'm fine if you want to change that now as part of this PR.

@azjezz
Copy link
Member

azjezz commented Jan 11, 2022

getX also gives the assumption that it always returns the same suspension.

DriverInterface::suspension(): SuspensionInterface - return, or create suspension for the current fiber

@kelunik
Copy link
Member Author

kelunik commented Jan 11, 2022

It does return the same instance, per fiber. It's similar to a ThreadLocal in Java.

@azjezz
Copy link
Member

azjezz commented Jan 11, 2022

yea, but I'm talking generally ( as between different fibers ), maybe DriverInterface::currentSuspension()?

@kelunik
Copy link
Member Author

kelunik commented Jan 11, 2022

@trowski @WyriHaximus Renamed and added a documentation comment that the driver must always return the same instance for each fiber.

@trowski
Copy link
Member

trowski commented Jan 11, 2022

+1 for keeping the old method and deprecating it during 0.2.x.

@kelunik
Copy link
Member Author

kelunik commented Jan 11, 2022

I've added EventLoop::createSuspension again to allow for a smoother transition from 0.1 to 0.2, so libraries can mostly require ^0.1 || ^0.2 instead of directly jumping to ^0.2.

* main:
  Also avoid re-creating callback fiber for throwing microtasks
  Avoid re-creating callback fiber on exceptions
  Remove fiber switches on callback fiber creation
@kelunik kelunik merged commit ab79812 into main Jan 14, 2022
@kelunik kelunik deleted the cache-suspensions branch January 14, 2022 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants