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 gigantic EqualsNode specializations #6191

Closed
JaroslavTulach opened this issue Apr 4, 2023 · 3 comments · Fixed by #6280
Closed

Any.== slowed down by gigantic EqualsNode specializations #6191

JaroslavTulach opened this issue Apr 4, 2023 · 3 comments · Fixed by #6280
Assignees
Labels
--low-performance -compiler -libs Libraries: New libraries to be implemented
Milestone

Comments

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Apr 4, 2023

#5898 investigation brought down the sieve benchmark to 518ms on my computer. While continuing investigations I found a way to get down to 333ms. It is just necessary to productize my prototype...

  • EqualsNode contains gigantic amount of @Specialization which leads to (possibly) too huge execute method in EqualsNodeGen. Such a huge method may overload the compiler thresholds and disable some optimization. @steve-s suggested to split the node into few smaller ones
  • We should optimize for most common cases - e.g. first of all check for primitive type comparisons and only then move to other builtins and/or to custom comparators
  • Current Any.== implementation checks comparators first - given most of our common cases don't have special comparator this is a waste of CPU

Remove the Enso Any.== code. Split the logic into multiple nodes. Revert the checks to optimize for most common situations. CCing @Akirathan

@JaroslavTulach JaroslavTulach added -compiler -libs Libraries: New libraries to be implemented --low-performance labels Apr 4, 2023
@JaroslavTulach JaroslavTulach added this to the Beta Release milestone Apr 4, 2023
@JaroslavTulach JaroslavTulach self-assigned this Apr 4, 2023
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board Apr 4, 2023
@Akirathan
Copy link
Member

I agree. Reducing Any.== to just a builtin method, without the overhead of some Enso code wrapper seems like a good idea, and will be easily implemented once we have #5740. Moreover, optimizing for primitive value comparison is indeed a good idea.

@Akirathan
Copy link
Member

EqualsNode contains gigantic amount of @specialization which leads to (possibly) too huge execute method in EqualsNodeGen. Such a huge method may overload the compiler thresholds and disable some optimization.

Right. Once there are many specializations instantiated, the compilation might get tricky. Also, it is a problem for the interpreter.

@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Apr 4, 2023
@JaroslavTulach JaroslavTulach moved this from 📤 Backlog to 🔧 Implementation in Issues Board Apr 14, 2023
@github-project-automation github-project-automation bot moved this from 🔧 Implementation to 🟢 Accepted in Issues Board Apr 17, 2023
@enso-bot
Copy link

enso-bot bot commented Apr 18, 2023

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

Progress: - EqualsNode got merged in: #6280

Next Day: Planning new interation. Fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--low-performance -compiler -libs Libraries: New libraries to be implemented
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants