Integrate improvements from Svelte-M into Svelte

[antoninadert]

Svelte-m was an experiment done by lega911 to improve performance and change detection in Svelte.
The experiment is conclusive and I believe we would all want the improvements to be integrated in Svelte.

On the other hand, if these changes were not integrated, I believe they would lead to the split of the svelte ecosystem in the midterm (as we have seen in Preact vs React for example).

This would be a big waste of time and energy so I would suggest that Svelte maintainers start the study to integrate the improvements into the baseline Svelte instead.

I hope this makes sense. Regards.

[kevmodrome]

How about making an RFC: https://github.com/sveltejs/rfcs

[antony]

@antoninadert Thanks for your issue. I had seen this on reddit earlier today and had put it on my reading list of things to investigate. I'm not sure if you intended it this way, but the way you have worded this issue comes across as extremely hostile, dogmatic, and almost threatening. Is there something that has given you cause to think that we'd for some reason be against adding optimisations into the compiler?

Either way, It seems that these improvements seem critical to you, so I would suggest as @kevmodrome has above, that you should raise an RFC to document specific optimisations you think should be added, along with the percieved benefit of such, so that people can start work on incorporating them and running benchmarks to determine what benefit these bring.

[opensas]

I think that this would solve one of the main shortcomings of Svelte's reactivity system, that is dealing with object references, as it's clearly stated in [the tutorial]:

const foo = obj.foo;
foo.bar = 'baz';

...won't update references to obj.foo.bar, unless you follow it up with obj = obj.

I stumbled with this a couple times. The easiest workaround is to add a obj = obj, like the tutorial says, or drive all modifications thru the observed variable (obj in this case).

Somehow this feels like the Achilles heel of Svelte reactivity system (which is gorgeous BTW), so if it could be handled in a more natural way it would be great.

On a side note, I also think that the issue wording has been a bit unfortunate, but it's great to see so many people playing with Svelte ideas (which not long ago were pretty radical and viewed with distrust by many) and trying to find ways to improve it. On the other hand, I have full confidence that svelte devs will find the way to take advantage of it to improve svelte, if that's the case.

Perhaps initially it could be implemented as an alternative reactivity system triggered by the <svelte:options>element, until it's found to be a valid alternative.

[gevera]

Indeed an interesting take on reactivity. Malina.js and the comparison of it to Svelte in this article. At least something to consider. I wonder if any of the ideas proposed could/should be implemented without much overhead

[antoninadert]

Hi @antony , this is not an agressive wording I intended, I just peacefully wrote my thoughts and the shortcomings I anticipated in case of following / not following the listed improvements, based on my little observations.

By no mean I was threatening or suggesting you would not be happy with the changes, I just reference a simple fact I observed before (Preact vs React example).

This is the first time I write such a thing on Svelte repo so I don't know you yet, hence I am just being factual. Next time I'll probably be kinder as it seems you are very welcoming people :)

On the other hand I don't have a rush to have these improvements, it's just that when I saw the original post about Svelte-M, I believed someone should raise this issue.
One week later it was still not there so I went ahead.
If I can share my intentions, I didn't want to put any pressure, just to explain my thoughts and raising this point.

[ansarizafar]

I strongly agree with @antoninadert.

[trbrc]

Could we get more specific about which improvements we're talking about? There are at least three changes I can see in Malina/Svelte-M:

1. Changes to some DOM operations, using innerHTML and cloneNode instead of building up the DOM element by element.
2. Different change detection, which is not explained very clearly that I can see - but from looking at the output, it appears like it is making shallow copies of objects to compare to when they might have changed.
3. Changes to syntax and semantics, such as allowing a statement as event handler, and only supporting references to arrays as the collection in {#each} blocks.

1 appears to be the easiest one for Svelte to support, assuming it was proven to be a performance improvement. I would love to see more convincing benchmarks.

For 2 I haven't found a good writeup of the principles behind the change detection, and it's not immediately clear to me from looking over the source. Spontaneously, I'm afraid that it would turn out to be very brittle, as it appears to rely on static analysis of what types different expressions would resolve to. But I might be misunderstanding. An RFC with a more detailed proposal would probably be necessary to evaluate the approach.

For number 3, it seems unrelated to most of the other things. I have no strong opinion, except that I would probably suggest the event handler shorthand used event and this instead of $event and $element, to stick closer to the default behavior of HTML event attributes.

[PatrickG]

For 3. there already is an issue #5087

[ansarizafar]

Svelte is great but its seems like its not moving forward. I am afraid If the situation remains the same then something like https://medium.com/@lega911/svelte-js-and-malina-js-b33c55253271 will take over. Its just a matter of time.

[trbrc]

Svelte 3 was a huge leap forward, just one year ago. It's not a good idea to have that type of leap all the time, as it is very expensive for users to keep up. As a user I prefer stability.

[j3rem1e]

For 1, there is this issue : #3898

For 2, the experiment looks like interesting, but after reading the source, It's looks like the digest cycle of angularJS :

• It keeps a list of expression to watch ;
• Everytimes "something" could have changed, it loops for every watch, evaluate the expression, and ifa change is detected, the associated variable is reassigned/callback executed
• Only primitive, or the first level of arrays seem to be dirty-checked
• I didntt success to use #each block, so I couldn't test array reactivity

It's a great toy-project, however I think there is a lot of works to make it really usable and scalable.

[ansarizafar]

Why not just use javascript proxy https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy like Vue 3

[RhettFF]

Wouldn't the safest way to do the reactivity change simply be to have the compiler add the "obj = obj" fix after a statement which changes the array or object? As humans, we know when it's needed, right? The compiler should be able to identify statements which would alter a variable without assignment.

[afaur]

@RhettFF "Wouldn't the safest way to do the reactivity change simply to have the compiler add the "obj = obj" fix after a statement which changes the array or object?"

• The compiler builds a ctx object that stores id to value.
• When you write the obj = obj the compiler adds code that invalidates that object by id.
• Heres an example of one way this may get complicated:

> let obj = { a: 1 }
> let key = await someValueFromAnApi() // Returns string of 'c' at runtime.

> obj
{a: 1}

> delete obj[key]
true

> obj
{a: 1}

• Since the compiler sees delete obj[key] should it invalidate obj? (obj = obj)
• Heres a repl that may explain/show this a little better:
  • https://svelte.dev/repl/ccc85a29ce4c4f61b1dc6e1841f389ae?version=3.23.2

[RhettFF]

@afaur Thanks for the example. It would be nice to "fix" the reactivity. It's a cause of confusion and it contorts the JS people write. I'm used to handling it now but it seems like a nail that's sticking up.

[afaur]

@opensas "

const foo = obj.foo;
foo.bar = 'baz';

...won't update references to obj.foo.bar, unless you follow it up with obj = obj."

What if obj contains that value to begin with (obj = { foo: { bar: 'baz' } })?
In that situation it would probably be better if everything that depends on that value did not redraw if it didn't have to.

The issue might be that the compiler does not know what the code is doing well enough to make that decision without adding runtime object checking code.

I don't fully understand what Svelte-M does, but maybe it adds code at compile time that looks through the object at runtime to see if anything has changed or not? If so, maybe it does get supported in svelte as an <option/> (as you suggested) so that people can opt in or out to the extra processing.

[gevera]

Wonder what @Rich-Harris has to say about this issue

And totally agree. Tiny, incremental changes are the way to go

Svelte 3 was a huge leap forward, just one year ago. It's not a good idea to have that type of leap all the time, as it is very expensive for users to keep up. As a user I prefer stability.

[opensas]

Why not just use javascript proxy https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy like Vue 3

I also thought when I first got hit by reactivity not picking object references, that js proxies could be a solution to fix reactivity without checking every possible change. Has anybody (more knowledgeable than me) evaluated such a solution?

[opensas]

For 1, there is this issue : #3898
[...]

* Everytimes "something" could have changed, it loops for every watch, evaluate the expression, and ifa change is detected, the associated variable is reassigned/callback executed

I guess that kind of solution is not compatible with Svelte's philosophy, next step would to implement virtual dom and diffing (:scream: !!!) Anyway, if implemented, I guess it should only be allowed as an opt int on a component basis.

[trbrc]

I also thought [...] that js proxies could be a solution to fix reactivity without checking every possible change. Has anybody (more knowledgeable than me) evaluated such a solution?

That's pretty much how Immer.js works, I believe. I don't think it's a very Svelte-y approach, since it's all runtime magic.

[RhettFF]

@afaur "What if obj contains that value to begin with (obj = { foo: { bar: 'baz' } })?"

I think it should be reactive in that case. After all, isn't Svelte reactive if a simple variable is updated with the same contents it already had?

[j3rem1e]

a change not documented in this implementation : you can't call methods with the reactivity label ($:). You have to use a special syntax and explicity declare your dependencies. it's probly due to the fact that the reactivity system has to evaluate every reactive expression on every digest cycle. it can't in the current implementation detect dependencies in method calls without side effect.

$: doSomething(myProperty); // not valid

$: myProperty, () => doSomething(myProperty); // valid

[dimfeld]

For 2, the experiment looks like interesting, but after reading the source, It's looks like the digest cycle of angularJS :

This was my impression as well. It's a nice demo and does solve some real problems. But having come from AngularJS development, I've seen the watcher model cause serious performance issues in larger applications, with little you can do about it.

A big draw of Svelte is that it figures out a lot of this stuff at compile time. In a highly dynamic language like Javascript doing this sort of analysis at compile time is difficult, at best, and often impossible to do comprehensively. As @afaur pointed out, aliasing is a real issue, and when you get into function parameters, asynchronous code, and so on, it gets even more difficult to track where and how certain pieces of data are changing.

Right now the system works in a reasonably predictable way IMO, and changing it to cover more cases runs the risk of adding additional edge cases where it becomes more difficult to figure out whether Svelte will count a statement as a change or not. I think any changes to Svelte's reactivity system should be carefully considered to not make things actually worse when you go beyond the simplest cases.

[Evertt]

That's pretty much how Immer.js works, I believe. I don't think it's a very Svelte-y approach, since it's all runtime magic.

@trbrc how do you mean it's all runtime magic? It's a super small proxy for each object, right? Like the only thing it's doing is something akin to:

let proxy = new Proxy({ /* original object */ }, {
    set: (obj, prop, val) => {
        if (obj[prop] !== val) {
            $$invalidate(/* dunno what goes here... */);
        }

        obj[prop] = val

        return true
    }
});

If a change this small could help to make things like array.push(element) work then I'd be all for it. :-D

[trbrc]

how do you mean it's all runtime magic? It's a super small proxy for each object, right?

I'm not necessarily against it - I think Immer is interesting, and proxies are obviously a powerful tool. But it doesn't seem to fit very well with the Svelte philosophy, which is to move the bulk of the work to the compiler.

If you like to use runtime magic, I think you can probably do it right now. For example, create a wrapper for a Svelte component that proxies all of its props, and re-renders when it detects a mutation. EDIT: Here's an experiment, using sindresorhus/on-change: https://svelte.dev/repl/67d54ba031d64a9a972ea4eedd727e93

If a change this small could help to make things like array.push(element) work then I'd be all for it. :-D

It looks simple, but it would probably require much more code to work well. For example, what about large, deeply nested objects? You would need to recursively wrap any accessed property values as well. Or consider what needs to happen to a proxied object if you pass it to an imported function. What if that function runs an equality check against the original, unwrapped value? That check would now fail.

[Evertt]

For example, what about large, deeply nested objects? You would need to recursively wrap any accessed property values as well.

I just did a quick google search and that seems relatively easy.

    // The following code
    let myObj = { name: 'Evert' }

    // Would turn into
    let myObj = new Proxy({ name: 'Evert' }, observer)

    // Where the observer is defined somewhere else,
    // just once, since it should be the same for all proxies.
    const observer = {
        get(target, key) {
            if (typeof target[key] === 'object' && target[key] !== null) {
                // This is here to make sure every nested object
                // will also be wrapped in a proxy object
                return new Proxy(target[key], observer)
            } else {
                return target[key]
            }
        },
        
        set(target, key, newValue) {
            if (target[key] !== newValue) {
                $$invalidate(/* invalidate target here... */)
            }

            target[key] = newValue

            return true
        }
    }

What if that function runs an equality check against the original, unwrapped value? That check would now fail.

Yes, that indeed seems like an important case to consider. Does Vue have an answer for that?

I understand what you say about it going against Svelte's current philosophy. I wonder if there's any significant performance penalty to doing these things at runtime with a proxy object instead of at compile time. Because it does make code way more intuitive if array.push(element) just works...

[trbrc]

It's not complete, now foo.bar === foo.bar might be false, and you would need to unwrap newValue before using it, in case that's proxied. You also need to wrap and unwrap method arguments and return values. It's probably possible to get it all fairly watertight, but I don't think it would look like just a small proxy anymore.

EDIT - sorry if we've derailed the thread. This proxy discussion should probably be a separate issue.

[Evertt]

Okay, touché 😅

…

On Tue, 7 Jul 2020 at 18:06, trbrc ***@***.***> wrote: It's not complete, now foo.bar === foo.bar might be false, and you would need to unwrap newValue before using it, in case that's proxied. You also need to wrap and unwrap method arguments and return values. It's probably possible to get it all fairly watertight, but I don't think it would look like just a small proxy anymore. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5103 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJVMUVGITN6BIZNLM3E37LR2NBYPANCNFSM4ORAAXVA> .

[afaur]

If a change this small could help to make things like array.push(element) work then I'd be all for it. :-D

For this specific use case we could always make array.push(element) become array[array.length++] = element at runtime.

Here are shift and pop (one liners that trigger reactivity):

• Shift: array = array.slice(1)
  • Update: https://jsperf.com/array-shift-vs-slice-1
• Pop: delete array[array.length--]

[Conduitry]

Using proxies has already been brought up in other issues, and is not something we're interested in changing now. Svelte integrates nicely with Immer if you want some of these benefits.

Making .push() and .pop() and friends on arrays trigger has also already come up in other issues. This can't be consistently checked for at compile time, and we don't want to introduce runtime checks for this. We'd much rather have simple rules about what triggers reactivity and what does not, than to have complicated rules for what's recognized as an update and what's not, which would gradually change over time as false positives and false negatives are fixed. Would these changes be considered bugs? Would they be considered features? Would an exhaustive characterization of the situations that the compiler recognizes as triggering reactivity become part of the documentation, and would it be kept updated?

Similarly, obj.foo = foo; foo.bar = baz; can't be consistently identified at compile time, and so faces the same issues as .push(), .pop(), etc.

If there are other concrete suggestions that are present in this other project or that have come up in this thread that you feel strongly about, please open well-scoped RFCs for them in this repo.

[samclaus]

If a change this small could help to make things like array.push(element) work then I'd be all for it. :-D

For this specific use case we could always make array.push(element) become array[array.length++] = element at runtime.

Here are shift and pop (one liners that trigger reactivity):

* Shift: `array = array.slice(1)`

* Pop: `delete array[array.length--]`

@afaur arr.slice(1) makes a second array while arr.shift() copies within the existing array and has less runtime overhead. #perfmatters

[samclaus]

Also, isn't the whole point of AOT compilation in Svelte and Angular (and maybe some other frameworks?) that it is extremely easy for the optimizing JS runtime to see hard coded variable names and make hidden classes? Wouldn't using proxies throw all that out the window and make the browser devolve to using hashmaps for property lookups?

[afaur]

@afaur arr.slice(1) makes a second array while arr.shift() copies within the existing array and has less runtime overhead. #perfmatters

• Added a jsperf to my original post, but the results seem to vary for me. (Tried v8 Chrome 83.0.4103)
  • https://jsperf.com/array-shift-vs-slice-1
  • It is worth noting that jsperf results will show ops per second, and not memory consumption.
  • shift operations may help memory consumption throughout a run, but I have not been able to verify that.
  • Current v8 implementations of shift and slice for more information: shift | slice

[afaur]

I was able to get the samples from the article to run locally today.

• I updated the timing measurements from the gist benchmarks to use performance.now and requestAnimationFrame.
  • Date.now was replaced with performance.now
  • Data update is measured as soon as the timed function is finished running.
  • setTimeout was replaced with requestAnimationFrame to measure paint.
• Original timeit function from the benchmark gist: MalinaJS | SvelteJS
  • Changes to timeit function: MalinaJS | SvelteJS
  • Deployed versions with these updates: Demo | Repo
• Update: I was able to adjust the example to time both the add to the array, and the paint. I will update the demo shortly.
• Update: The paint is where the performance deviates largely between Svelte-M / MalinaJS and SvelteJS v3.23.2
  • Both the demo and the repo have been updated: Demo | Repo
• Below is an example run of both examples in v8 Chrome 83.0.4103 using performance.now with paint measured.
  • Svelte-M / MalinaJS
    • Writing 5000 items to an array: Between 1.58ms and 1.75ms
    • Painting 5000 items: Between 0.32ms and 0.38ms
    • Remove array item (Array splice using indexOf): Between 0.02ms and 0.03ms
    • Painting after removal: Between 0.28ms and 0.35ms
  • SvelteJS v3.23.2
    • Writing 5000 items to an array: Between 1.78ms and 1.84ms
    • Painting 5000 items: Between 916.65ms and 1877.00ms
    • Remove array item (Array splice using indexOf): Between 0.04ms and 0.06ms
    • Painting after removal: Between 116.00ms and 118.06ms
  • It may be helpful to ask the author to update the article with performance.now timings.
• paint seems to be where svelte may need some attention.
  • I will look at Node.cloneNode, and building the DOM as one command as mentioned in the article.

[patrickjquinn]

Give @afaur previous findings, did anything ever happen around this? The potential paint improvements alone should be cause for great interest surly?