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

FEAT: scalable metered reactivity #4529

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

hiiamboris
Copy link
Collaborator

@hiiamboris hiiamboris commented Jun 17, 2020

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 than select 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 off
  • true 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 to yes 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.
  • when it can find an object inside the global context, it will refer to it by word, not by the massive 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 start
  • system/reactivity/metrics/show prints the full statistics, e.g.:
***** REACTIVITY METRICS REPORT *****
Metrics collection enabled?: true
Statistical counts:
    events triggered:    80015
    reactions fired:     50000 (immediately: 25005 , queued: 24995 )
    reactions skipped:   0
Time spent in reactions: 0:00:00
Time spent in reactivity:
    total:               0:00:09.55855
    adding relations:    0:00:03.80822 (preparations: 0:00:00.901052 )
    removing relations:  0:00:00.843048
    dispatching:         0:00:04.90728
    longest queue flush: 0:00:00.0020001
Peak values:
    maximum queue size:  5000
    maximum index size:  5000
    biggest relation:    2 reactors
    most used reactor:   5000 relations

I also made is much smarter:

;-- `is` should support non-object paths, like `pair/x`, `time/second`, `block/3`
;; as well as in-path references: `anything/(pair/x)`, `any/:x/thing`...
;; parsing summary:
;; word: as first item in a path only (including inner paths)
;; get-word: anywhere in a path, inside parens (including inner paths) e.g. `object/:x`
;; get-words in lit-paths and set-paths? for now they are accepted; e.g. `obj/:x: y`
;; lit-word: should be ignored? as it's a way to get around reactivity e.g. `set 'y get 'x`
;; interop with react: react catches words after the object, not get-words; is - only first word in path

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

Copy link
Contributor

@greggirwin greggirwin left a 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.

environment/reactivity.red Outdated Show resolved Hide resolved
environment/reactivity.red Outdated Show resolved Hide resolved
environment/reactivity.red Outdated Show resolved Hide resolved
@hiiamboris
Copy link
Collaborator Author

hiiamboris commented Jun 27, 2020

Another side effect of this PR - less GC load during face creation.
disregard this, it seems totally random (varies by 500% from time to time)

@hiiamboris
Copy link
Collaborator Author

Last commit makes tight (r/x: r/x + 1) loops ~5% slower but reduces reactor's memory consumption by a factor of four: from 5.7kB to 1.4kB (on master branch it's 4.5kB).

@greggirwin
Copy link
Contributor

Thanks for continuing work on this. I hope @dockimbel will be able to review soon.

tab "new :" type? :new
]
#local [
#macro REL-PERIOD: func [][3]
Copy link
Contributor

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.

Copy link
Collaborator Author

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] [
Copy link
Contributor

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.

Copy link
Collaborator Author

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! :)

Copy link
Collaborator Author

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.

@qtxie qtxie force-pushed the master branch 3 times, most recently from 1ec06cd to 7218fb4 Compare October 7, 2021 11:57
@hiiamboris hiiamboris mentioned this pull request Jan 7, 2022
@hiiamboris
Copy link
Collaborator Author

I'd like to make Spaces reactive, but this has been ignored for almost two years already.

@greggirwin
Copy link
Contributor

I think @dockimbel has looked at it, but wasn't sure about the design. Please weigh in when you can @dockimbel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants