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
FEAT: scalable metered reactivity #4529
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great writeup. I appreciate all the included tests and metrics.
|
Last commit makes tight ( |
Thanks for continuing work on this. I hope @dockimbel will be able to review soon. |
environment/reactivity.red
Outdated
tab "new :" type? :new | ||
] | ||
#local [ | ||
#macro REL-PERIOD: func [][3] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Macros seem like overkill here, even if simple, compared to just int refs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? They will become integers during compilation.
TBH I just didn't want them to be "variables", as that implies one could change their values, so I used macro as a sort of constant declaration, same as R/S code does throughout. (besides, I suppose compiled integers give it a tiny bit more juice :D)
|
||
reactor: function [spec [block!]][make reactor! spec] | ||
deep-reactor: function [spec [block!]][make deep-reactor! spec] | ||
--debug-print--: function [blk [block!] /full /no-gui] [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer non-decorated names. The later sr/--debug-print--
calls look odd. Will have to see what @dockimbel thinks on special func naming conventions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started with undecorated names only to find out that I am unable to tell apart actual code that does the job from supplemental metering and debugging code, so I changed it to the way it is.
Propose better names, but please test how they look first in the editor, e.g. for check
func. If it's cleaner I'm all for it! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more sr/
prefix in the recent update.
1ec06cd
to
7218fb4
Compare
I'd like to make Spaces reactive, but this has been ignored for almost two years already. |
I think @dockimbel has looked at it, but wasn't sure about the design. Please weigh in when you can @dockimbel. |
TL;DR: ~10x faster reactivity with O(1) complexity and multiple fixes here.
Rationale.
Since hash lookups are very cheap compared to Red token evaluation time, I decided to scratch the idea of putting relations into
on-change*
and use a central reactors index (hashtable). As I expected it's even faster thanselect body-of :on-change* relations
approach I used before. Tests show further 20% speedup, but the number is not reliable as I'm on another PC which may have better optimized CPU cache.Performance
Main source of speedup in this PR is avoiding O(n) lookup times by splitting the unstructured index into 2:
reactor [relations ...]
reaction [source-objects ...]
All operations become O(1) and it really scales.
You can see the benchmark results here: for reactors, for faces . I expect ~20-30% better timings with the fix for #4505 (it was fixed in this PR's snapshot).
There's still slight linearity O(n) for block reactions due to #4466 - once it is fixed I expect it to be nearly O(1) up to huge loads.
Other notes
Shallow reactors do not take ownership of assigned series anymore, so if some series is watched by a deep reactor, it can now safely be assigned to any number of shallow reactors
Objects not based on reactor! template can still use
reactivity/check
to embed themselves into reactivity framework (this part was also true in the original design)I've also come to realize that
reactivity/check
should return blazingly fast for objects without relations.E.g. in my Spaces project I'd like to make objects possibly reactive, but there may be thousands of objects, 99% of which won't be reactive, only a few top-level ones. And I need performance cost of adding reactivity on top of it to be negligible.
This implementation satisfies that need.
Bonus features are: better debugging output and the ability to collect statistics.
Debugging summary:
system/reactivity/debug?
now can be:off
/none
to turn debug output offtrue
or any other truthy value to turn it on for most interesting output, namely added, removed, skipped (most valuable!) and queued reactions'full
(word) adds toyes
a more extensive output: each fired reaction, checks for reactions, on-change events. Note: part of this output won't be shown in GUI console (as it'll crash with stack overflow). Use CLI console to see all the details.object [....]
output, so it's much easier to tell what's what.Metering summary:
system/reactivity/metrics?: true
turns on stats collection,false
turns it off. Note: metering (when on) slows it down by ~20%, so use only when you need it.system/reactivity/metrics/reset
zeroes all stats for a fresh startsystem/reactivity/metrics/show
prints the full statistics, e.g.:I also made
is
much smarter:red/environment/reactivity.red
Lines 302 to 309 in a77d72c
I have a lot of notes on pitfalls, tricks. If this PR gets approved, I will write them down to a wiki.
And along the way:
Fixes #4510, fixes #4507, fixes #4471, fixes #4176, fixes #4166, grants wish REP#75