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

added xania's implementation to the bench #13

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

xania
Copy link

@xania xania commented Dec 29, 2022

No description provided.

@xania
Copy link
Author

xania commented Dec 29, 2022

Hi @mihar-22, thank you for making this benchmark possible, it immediately pointed out fuctional and performance issues in my library that I needed to fix.
The implementation of @xania/state is inspired by rxjs.

I ran the benchmark on my machine many times and the conclusion is the same every time. Somehow is maverick only faster on the first column.

image

I also don't understand how preact's implementation is much faster.

@mihar-22
Copy link
Member

mihar-22 commented Dec 29, 2022

Hey @xania!

There could be a few reasons it's faster:

  1. It's not feature equivalent. We have a separate scheduler, roots, effect roots, effect nesting, scope tracking, and few other things. This is all overhead and it's why I've removed certain libraries from the bench inside this repo.

  2. The CellX benchmark only looks at your library through a very narrow lens. It's mainly static and respects eagerly queueing operations and aggresively caching. It's a poor benchmark for signals libraries as we want to understand more of the dynamic behaviour. I recommend checking out a few more such as Reactively and the creation cost by S.js - I linked the to Solid adaptation.

  3. I've made this mistake of thinking I've made something really fast but it turns I wasn't handling the most important part - cleanups! This can be a huge memory leak and make the library unusable, so really make sure that everything is cleaned up correctly. From a quick look at Xania I can't see any clean up operations (maybe I missed it) but that could be why. You should notice a considerable slow down after implementing it.

  4. Important to measure what trade off you're making for speed - memory or size? How much have you traded off and is it suitable for your needs.

  5. Do you support dynamic sources/observers? For example, if a computed contains the following the sources A -> B -> C on the first run, and A -> C on a subsequent run, will it still update if B updates - preferably it shouldn't. This is where a good set of testing can help you understand how you're implementation is handling different graphs.

@xania
Copy link
Author

xania commented Dec 30, 2022

@mihar-22
He Rahim, my name is Ibrahim by the way.

  1. Clearly I am new to these concepts. As xania is implementing the observable pattern I need to ask, are those generic reactivity features or are those specific to implementations using signals concept. preferably the tests should be able to validate the correctness of these features in a library, which gets me thinking, would you be open for PR's on that aspect?
  2. Will check
  3. Xania distinguishes between
    a. constructing the graph modeling the flow of data, because the graph is just a local object, clean up is handled by garbage collector.
    b. observing the data at the nodes that flows through the graph. you can subscribe to it which returns unsubscribable object for clean up. because nodes of the graph are hot observables (cache the data) we dont need any additional observers for the purpose of this benchmark to get the data from the graph.
  4. Xania is optimized for both size and speed thanks to the graph algorithm used.
  5. In Xania you can only compose to construct new graphs. Or create new graph from scratch which is usually not in the critical path. xania is not trying to optimize for this I guess because it's not solving uses cases I am familiar with. Can you think of a real world use case where one needs to remove a node B in the middle of the graph after constructing it (maybe animation?)

@xania
Copy link
Author

xania commented Dec 30, 2022

I've found some more perf optimizations
image

How is 10 almost double the 100 for xania? So I also tried more smaller test cases to understand the seemingly relative high overhead of xania in those cases but then I got these results:
code can be found here

image

results for less then 10 are kind random on my machine at least. I also added the total column which is perfectly stable in determining the rank of libs.

But it gets more confusing. when I just flip the tiers then I get these results
image

Seems like a conditional warming up issue, but only when the first case is 10 and the lib is xania??

@milomg
Copy link

milomg commented Jan 5, 2023

  1. In Xania you can only compose to construct new graphs. Or create new graph from scratch which is usually not in the critical path. xania is not trying to optimize for this I guess because it's not solving uses cases I am familiar with. Can you think of a real world use case where one needs to remove a node B in the middle of the graph after constructing it (maybe animation?)

It turns out that this is really useful for UI libraries, such as when hiding (and removing) element from the DOM.

For example, Solid implements a Show component by doing something like createMemo(() => condition() ? a() : b());. Two things are important for this: when condition() goes from true to false any elements created in b() need to be destroyed (cleanups), and then the entire memo should only update if a()'s deps change, not if b()'s deps change (unsubscribe from b()).

@xania
Copy link
Author

xania commented Jan 5, 2023

That's reasonable, it still aligns with the design I had in mind because we cannot just randomly remove a node in the graph. But when we do remove a node like in your example that node is well defined as part of the graph definition. And because of that I can add that as a feature without impact on perf.

@xania
Copy link
Author

xania commented Jan 8, 2023

  1. In Xania you can only compose to construct new graphs. Or create new graph from scratch which is usually not in the critical path. xania is not trying to optimize for this I guess because it's not solving uses cases I am familiar with. Can you think of a real world use case where one needs to remove a node B in the middle of the graph after constructing it (maybe animation?)

It turns out that this is really useful for UI libraries, such as when hiding (and removing) element from the DOM.

For example, Solid implements a Show component by doing something like createMemo(() => condition() ? a() : b());. Two things are important for this: when condition() goes from true to false any elements created in b() need to be destroyed (cleanups), and then the entire memo should only update if a()'s deps change, not if b()'s deps change (unsubscribe from b()).

It turns out this is achieved by monadic bind <T, U>(S<T> source, f: (x: T) => S<U>) => S<U>

Example

const a = new State(1);
const b = new State("even");
const c = new State("odd");
const d = a.bind(x => x % 2 === 0 ? b : c);

In this example d depends on a and conditionally on b. if the condition changes than all connection to b are cleared and new connection is added to c.

@xania
Copy link
Author

xania commented Jan 12, 2023

@mihar-22
Hi Rahim, I have added all the features for better comparison, I also think it is useful to have a total column in the score table.

@mihar-22 mihar-22 force-pushed the main branch 2 times, most recently from 6ebfe0f to 059e054 Compare July 2, 2024 06:23
@mihar-22 mihar-22 force-pushed the main branch 2 times, most recently from 8aae6af to 2c2a048 Compare November 27, 2024 07:38
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.

3 participants