haxetink / tink_state

Handle those pesky states.
The Unlicense
29 stars 13 forks source link

V1 #40

Closed back2dos closed 4 years ago

back2dos commented 4 years ago

Mostly, this is about drastically simplified ObservableMap / ObservableArray. Attempts to fire changes granularly had way too much overhead and are relatively useless given that auto observables and bindings are good at avoiding unnecessary work. As a result, the data structures are much simpler and MUCH faster, e.g.:

haxe.Timer.measure(() -> for (i in 0...10000) {
  var o = [];
  for (i in 0...1000) o.push(i);
}); // 0.09100008010864258s

haxe.Timer.measure(() -> for (i in 0...10000) {
  var o = new tink.state.ObservableArray();
  var link = Observable.auto(() -> {
    var sum = 0;
    for (x in o.values())
      sum += x;
    sum;
  }).bind(function () {});
  for (i in 0...1000) o.push(i);
  link.cancel();
}); // 0.42799997329711914s

haxe.Timer.measure(() -> for (i in 0...1000) {
  var o = new tink.state.legacy.ObservableArray();
  var link = Observable.auto(() -> {
    var sum = 0;
    for (x in o.values())
      sum += x;
    sum;
  }).bind(function () {});
  for (i in 0...1000) o.push(i);
}); // 3.3919999599456787s ... not that this is 1/10 of the iterations

It's about 4 times slower than plain arrays and about 100 times faster than the previous version.

There's still some room for optimization, but my primary concern for now is proceeding with any breaking changes necessary before this gets to be a 1.0.0.

kevinresol commented 4 years ago

Under this new implementation I will have to manually implement the signals if I want to observe the add/remove activities?

back2dos commented 4 years ago

How granular do you need the change notifications to be?

kevinresol commented 4 years ago

Currently I have some code that directly used the changes signal to listen on the add/remove events of a Map. But now I guess I should really re-think that part...

back2dos commented 4 years ago

Hmm, well, that's also possible. I don't mind adding some more granular notification, but I'd like to understand the use case to hit a decent sweet spot between flexibility and performance ;)

kevinresol commented 4 years ago

In a nutshell it is a Bluetooth LE library, I need to keep track of a list of currently visible peripherals nearby, and I need to be able to perform actions when one is discovered or gone.

https://github.com/why-haxe/why-ble/blob/6f15f96624532df140291a035f34938326333a84/src/why/ble/Central.hx#L71-L79

back2dos commented 4 years ago

Hmm, I see. Well, the more immediate problem is that you can't subclass ObservableMap, because it's an abstract now ^^

back2dos commented 4 years ago

@kevinresol Do you need the subclassing? It would be possible to expose the implementation and have something like this:

@:forward
abstract ObservableMapOver<K, V, Map:IMap<K, V>> {}
typedef ObservableMap<K, V> = ObservableMapOver<K, V, ObservableMapImpl<K, V>>;
// userland
class MapWithSparklesImpl<K, V> extends ObservableMapImpl<K, V> {}
typedef MapWithSparkles<K, V> = ObservableMapOver<K, V, MapWithSparklesImpl<K, V>>;

Not a huge fan though, especially since allowing inheritance always increases potential for conflict ^^

kevinresol commented 4 years ago

I don't think I liked subclassing but I guess I did so to get access to the private change signal. I think I can rework it.