tonsky / datascript

Immutable database and Datalog query engine for Clojure, ClojureScript and JS
Eclipse Public License 1.0
5.45k stars 304 forks source link

prevent explosion of redundant tuples during rule solving #457

Closed RutledgePaulV closed 9 months ago

RutledgePaulV commented 10 months ago

Addresses performance problem reported in https://github.com/tonsky/datascript/issues/456

Prior to this change tuples contained a bunch of duplicates due to calling -collect instead of collect which does deduplication. Those duplicates then propagated across the relations causing an unnecessary amount of work.

latacora-paul commented 10 months ago

Oops, just realized the test isn't cljs compatible and there are some broken tests when run under cljs, will fix.

RutledgePaulV commented 9 months ago

In the latest commit I descoped the fix for ClojureScript and because it's more complex. All tests now pass on both platforms.

I'll propose we apply the issue on the JVM first since it's pretty straightforward and perhaps @tonsky can help with the ClojureScript side since it's non-obvious to me why the same fix doesn't work for both. I could see an argument for "fix the compatibility issues" so that the same performance fix works for both, but there may be good reasons for why they're divergent and need separate solutions.

tonsky commented 9 months ago

I think cljs was not expected all tuples (arrays) to be converted to vectors just yet. Thanks for the contribution! Pushed 1.6.0

tonsky commented 9 months ago

1.6.1 :)