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.

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

Remove double-boxing of Computation #31

Merged
merged 1 commit into from
Mar 9, 2021
Merged

Conversation

Kestrer
Copy link
Contributor

@Kestrer Kestrer commented Mar 9, 2021

Computation previously had two boxes (or well box-likes) as it was essentially an Rc<Box<dyn Fn()>> - so I replaced this with just an Rc<dyn Fn()>. Additionally, I switched from a Vec<Rc<Computation>> to a HashSet<Computation>, which should allow observe to become a constant-time instead of linear-time operation.

@Kestrer
Copy link
Contributor Author

Kestrer commented Mar 9, 2021

Oh no, CI failed because I don't have permission to push to GH pages. Is there a way to fix that?

@lukechu10
Copy link
Member

Oh no, CI failed because I don't have permission to push to GH pages. Is there a way to fix that?

Yeah, I messed up the trigger for the GitHub pages action. Just ignore it. Should be fixed in #29

I can merge this but there will probably be conflicts with #29 but I'll deal with that.

Copy link
Member

@lukechu10 lukechu10 left a comment

Choose a reason for hiding this comment

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

LGTM

@lukechu10 lukechu10 merged commit e5dddbc into sycamore-rs:master Mar 9, 2021
@lukechu10 lukechu10 added the performance Performance related label Mar 9, 2021
lukechu10 added a commit that referenced this pull request Mar 10, 2021
commit b61242a
Author: Luke Chu <37006668+lukechu10@users.noreply.github.com>
Date:   Tue Mar 9 22:14:18 2021 -0800

    Create CODE_OF_CONDUCT.md (#33)

commit e5dddbc
Author: Kestrer <kestrer.dev@gmail.com>
Date:   Tue Mar 9 18:44:56 2021 +0000

    Remove double-boxing of Computation (#31)
lukechu10 added a commit that referenced this pull request Mar 10, 2021
* Refactor Signal

Make Signal a normal struct rather than a tuple struct

* Rename Computation to Callback

* Rename DEPENDENCIES to CONTEXT

* Do not trigger build_docs on pull_request

* Use a trait object to store dependencies

* Rename CONTEXT to RUNNING

* Move subscribe closure into subscribe_callback

* Build on push

* Make CONTEXTS a Vec stack

* Add (ignored) test for effect dynamic dependencies

* Get rid of execute()

* Store effect state in Running

* Add cleanup function

* wip

* wip

* Replace Box with Rc

* Replace Vec with HashSet

* Make dependency use a HashSet

* Add doc comment to cleanup_running

* Squashed commit of the following:

commit b61242a
Author: Luke Chu <37006668+lukechu10@users.noreply.github.com>
Date:   Tue Mar 9 22:14:18 2021 -0800

    Create CODE_OF_CONDUCT.md (#33)

commit e5dddbc
Author: Kestrer <kestrer.dev@gmail.com>
Date:   Tue Mar 9 18:44:56 2021 +0000

    Remove double-boxing of Computation (#31)

* Refactor create_effect_initial to recreate dependencies

* Refactor create_effect using create_effect_initial
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants