Skip to content
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.

Sign up for GitHub

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

Jump to bottom

A couple of minor improvements to AsyncContextFrame::StorageKey #540

Merged
merged 1 commit into from
Apr 20, 2023
Merged

A couple of minor improvements to AsyncContextFrame::StorageKey #540

merged 1 commit into from
Apr 20, 2023

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 13, 2023

  • Reset the tracing storage key in IoContext when the context is destroyed.
  • Do not store a key if the key has already been reset
  • Compare pointers and not hash in operator=

@jasnell jasnell force-pushed the jsnell/async-context-storagekey-improvements branch from 6073eeb to a8e096e Compare April 18, 2023 16:37
@jasnell
Copy link
Member Author

jasnell commented Apr 18, 2023

Updated to remove the AtomicRefcounted change (yay!). Also rebased. Please take another look!

jclee
jclee approved these changes Apr 18, 2023
View reviewed changes
src/workerd/jsg/async-context.h Show resolved Hide resolved
@jclee
Copy link
Contributor

jclee commented Apr 18, 2023

Weird... looks like the ubuntu and macos CI builds timed out, and the windows build failed early on with a 404 while trying to pull something. Will try rerunning.

@jclee
Copy link
Contributor

jclee commented Apr 19, 2023

(Rerunning the builds seems to have succeeded.)

@jasnell jasnell force-pushed the jsnell/async-context-storagekey-improvements branch 2 times, most recently from 5ac2c93 to 1487d2a Compare April 20, 2023 13:36
@jasnell
A couple of minor improvements to AsyncContextFrame::StorageKey
f8e7360 
* Reset the tracing storage key in IoContext when the context is
  destroyed.
* Do not store a key if the key has already been reset
* Compare pointers and not hash in operator=
@jasnell jasnell force-pushed the jsnell/async-context-storagekey-improvements branch from d574220 to f8e7360 Compare April 20, 2023 13:46
@jasnell jasnell merged commit ee9346d into main Apr 20, 2023
@jasnell jasnell deleted the jsnell/async-context-storagekey-improvements branch April 20, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kentonv kentonv kentonv left review comments

@jclee jclee jclee approved these changes

@harrishancock harrishancock Awaiting requested review from harrishancock

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jasnell @jclee @kentonv