Skip to content

Conversation

@poke1024
Copy link
Contributor

This is continuation of #575 and #615. It's my cleaned up proposal for fixing quadratic runtimes in Mathics.

The following plots show the results of the accompanying two commands, run once using master (yellowish) and once using this PR (blue). Don't ask me where the spurious regular dots come from (garbage collection?).

First[Timing[Fold[#1+#2&, Range[#]]]]& /@ Range[750]

plot-1

MyTotal[x0_] := Module[{x = x0, y = 0}, While[Length[x] > 0, y = y + First[x]; x = Rest[x]]; y];
First[Timing[MyTotal[Range[#]]]]& /@ Range[500]

plot2

The changes are twofold:

The quadratic runtimes are only fixed when both measures are applied, which is why I merged everything into this PR now.

All the information regarding cached Expression information (including what is Expression._sequences in master) is now bundled in ExpressionCache, which makes the code much cleaner I think.

I experimented with quite a variety of data structures, including dicts instead of sets for the symbol caching, but a set and a sequences list seems to be the best thing to do.

@GarkGarcia GarkGarcia added the needs rabase An old PR that needs to be rebased label Sep 9, 2020
@GarkGarcia
Copy link
Contributor

GarkGarcia commented Sep 9, 2020

@mmatera This seems related to #844

@mmatera
Copy link
Contributor

mmatera commented Sep 17, 2020

I am trying to do a rebase of this PR, but there are something that I do not completely understand: in Expression, in the PR, leaves are represented by tuples. This originates some errors in operations that try to modify the leaves of an expression. I do not understand the motivation for using tuples instead of lists. Is about safety in the code? or about performance? @poke1024 could you give me a hand with this?

(please see the merge at https://github.com/mmatera/Mathics/tree/qtime)

@rocky rocky mentioned this pull request Oct 2, 2020
rocky added a commit that referenced this pull request Oct 2, 2020
Rebase of #619. See that for more detail.
@rocky
Copy link
Member

rocky commented Oct 2, 2020

#936 should be the same thing rebased on current sources.

@rocky rocky closed this Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs rabase An old PR that needs to be rebased performance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants