-
Notifications
You must be signed in to change notification settings - Fork 9
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
cartesian product slow enough to be mistaken for a hang #623
Comments
not sure if I'd call this a bug. But some optimization is needed for sure. |
It seems a lot of the time is spent in calling the lambdas for |
I already removed redundant initialization but that didn't seem to make much of a difference. |
Huh, on a totally unrelated note, if I change |
Actually |
testing this with 40 takes 6 seconds now on my machine. Up to 101 still takes more than a minute (77 seconds). But please try this again @jvasileff and let me know what you think. |
Whoa, huge improvement! ftr, here are some (rough) measurements:
|
@chochos I don't quite understand why we have 38s for this. Surely there must be a way to get rid of the cruft? |
yes |
What I did last was optimize the native tuple implementation; instantiation was taking too long. I already checked the iterable I use for sequenced arguments and there's no obvious bottleneck there (that's how I got to the tuple, in fact). The callables need to be wrapped in case of a spread args call (as in JVM). Yes the performance sucks but I haven't found a way to make all that stuff about spreading args/flattening/unflattening work fast. |
Ok, this is a bit insane (and slightly off topic, sorry). I compiled the Dart output to JS using Note: the timing isn't really fair since the Dart backend doesn't have reified generics. The proof:
|
Kewl :) |
So can we compare the produced code between Dart-js and our js? Is the cost paid for reified generics? |
I don't see anything obvious that would call flatten/unflatten in the original code. I don't think it calls it on the JVM. |
In other words: why would map and filter do anything special wrt spread/tuple arguments? |
If @chochos is interested I can make it available. It's 3.4MB and includes a significant portion of the language module. |
it's not that map and filter do anything special about that. It's that any function reference can be called with spread or tuple args. That's easier to check on the JVM than on JS |
@jvasileff mine's only 3K (without language module of course), plus 1K for the model. |
On the JVM functions can only be called one way. You only pay for spreading when you spread. Can't you do the same? |
Haha, yeah. For the language module in particular, each To be fair, the simple.dart module is 21K including my benchmark utility. |
So one of these tests will always fail in js: shared void run() {
value echoFirst = (Object+ xs) => xs.first;
assert (echoFirst([[""]]) == [[""]]);
assert (echoFirst(*[[""]]) == [""]);
} |
hmmm that sounds like a separate bug. Something to do with that damn wrapper for spreading. |
OK so that latest example works now @jvasileff but I'm still searching for ways to optimize function ref calls. |
Unless you can pull a magic trick and make it FTL, shall we move this to 1.3? |
definitely not FTL but I'm (was?) working on something that reduces the time by half. But I'm afraid it requires moving around a lot of stuff related to callable references and wrapping callables etc, and we need to maintain backwards compat, so yeah moving this to 1.3 seems the most prudent thing to do at this point. |
Cool. |
This program seemingly takes forever to finish on js:
The
x
function is an obtuse way to generate a sequence of wrapped integers in the range0..101
.Adjusting
101
to a lower value, such as40
, does produce results prior to an overwhelming urge to press ctrl-c.The similar program below runs quickly:
The text was updated successfully, but these errors were encountered: