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

Any.== slowed down by Panic.catch 2x times #5898

Closed
JaroslavTulach opened this issue Mar 13, 2023 · 3 comments · Fixed by #6184
Closed

Any.== slowed down by Panic.catch 2x times #5898

JaroslavTulach opened this issue Mar 13, 2023 · 3 comments · Fixed by #6184

Comments

@JaroslavTulach
Copy link
Member

I have just been verifying the effect of #5845 on performance of sieve.enso and via VisualVM Polyglot Sampler I can see that an enormous time is being spent on line 31 comparing == 0 and subsequently catching panics. Single round of the benchmark takes:

Hundred thousand prime numbers in 895 ms. Last one is 1299709

By changing the Enso standard library to:

diff --git distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso
index 565b64adcb..7181423508 100644
--- distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso
+++ distribution/lib/Standard/Base/0.0.0-dev/src/Any.enso
@@ -108,8 +108,8 @@ type Any
     == self that =
         # If there is No_Such_Conversion, then `self` and `that` are probably
         # host or polyglot values, so we just compare them with the default comparator.
-        eq_self = Panic.catch No_Such_Conversion (Comparable.from self) _-> Default_Comparator
-        eq_that = Panic.catch No_Such_Conversion (Comparable.from that) _-> Default_Comparator
+        eq_self = Comparable.from self
+        eq_that = Comparable.from that
         similar_type = Meta.is_same_object eq_self eq_that
         case similar_type of
             False -> False

I am able to get to better numbers. Single round of the benchmark goes down to:

Hundred thousand prime numbers in 403 ms. Last one is 1299709

e.g. the benchmark gets twice as fast by eliminating Panic.catch call.

@hubertp
Copy link
Collaborator

hubertp commented Mar 13, 2023

Isn't this related to the whole if/then/else slowdown. Seems like thunk execution is really the root of all evil.

@jdunkerley jdunkerley added this to the Design Partners milestone Mar 14, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Mar 14, 2023
@jdunkerley jdunkerley moved this from 📤 Backlog to ❓New in Issues Board Mar 14, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Mar 14, 2023
@JaroslavTulach JaroslavTulach moved this from 📤 Backlog to 🔧 Implementation in Issues Board Apr 3, 2023
@JaroslavTulach
Copy link
Member Author

Isn't this related to the whole if/then/else slowdown. Seems like thunk execution is really the root of all evil.

Yes, the if/then/else handling may be the cause. I did some investigation #5709 (comment) and yes, there is a spurious OptimizedCallTarget.callBoundary - it shouldn't be there. However I am not sure how to get rid of it.

Anyway the current Any.== logic is complex and simplifying it for simple cases is also desirable.

@JaroslavTulach JaroslavTulach moved this from 🔧 Implementation to 👁️ Code review in Issues Board Apr 4, 2023
@mergify mergify bot closed this as completed in #6184 Apr 4, 2023
mergify bot pushed a commit that referenced this issue Apr 4, 2023
Fixes #5898 by removing `Catch.panic` and speeding the `sieve.enso` benchmark from 1058 ms to 514 ms. Should there be no dedicated conversion, let's use one defined on `Any` type - e.g. defining a conversion `from(Any)` makes such a conversion is always available.
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Apr 4, 2023
@enso-bot
Copy link

enso-bot bot commented Apr 5, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-04-04):

Progress: - Any.== integrated: #6184

Next Day: Lazy atom fields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants