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

Remove variables not referenced by input or output variables #1

Closed
ToddFincannon opened this issue Jan 24, 2017 · 8 comments · Fixed by #44 or #190
Closed

Remove variables not referenced by input or output variables #1

ToddFincannon opened this issue Jan 24, 2017 · 8 comments · Fixed by #44 or #190
Assignees
Labels
Milestone

Comments

@ToddFincannon
Copy link
Collaborator

No description provided.

@travisfranck
Copy link
Collaborator

Just noting that sometimes there are used to calculate a UI variable. Not sure this is critical feature to implement, and could remove a variable expected for UI.

@ToddFincannon
Copy link
Collaborator Author

Yes, we tried this and that turned out to be an issue. A better approach would be a more general "tree-shaking" algorithm that works backward from the list of output variables and drops everything that is not a dependency. That would include display variables and also drop variables that are not contributing to outputs we are interested in.

@travisfranck
Copy link
Collaborator

travisfranck commented Jul 24, 2017 via email

@ToddFincannon
Copy link
Collaborator Author

When we generate the model in SDEverywhere, we give a list of input and output variables in the spec JSON file. All the graph and table variables appear as output variables.

Memory allocation time is almost certainly not affecting our performance much compared to calculations. It is also a one-time startup cost.

@travisfranck
Copy link
Collaborator

travisfranck commented Jul 25, 2017 via email

@ToddFincannon
Copy link
Collaborator Author

ToddFincannon commented Jul 25, 2017 via email

@chrispcampbell chrispcampbell changed the title Remove variables marked as supplementary Remove variables not referenced by input or output variables Sep 10, 2020
@chrispcampbell
Copy link
Contributor

I'm reviving this old issue and changing the title to align with Todd's notes in this comment.

Recently I was trying to process a larger model using SDE and noticed that SDE wasn't already eliminating variables that are not relevant to the set of input and output variables from the spec file. SDE already does most of the reference tracking that we need for this. To make it complete, we just need to fix a couple places where references were not being recorded, and then add a step just before code generation to eliminate variables that are not in the set of active references.

@chrispcampbell chrispcampbell added this to the 0.6.0 milestone Sep 10, 2020
@chrispcampbell
Copy link
Contributor

Here are the numbers that show the benefits of this change using the En-ROADS model (v2.7.29) as a real world test case, and relative to other recent performance work. The benefits depend on how many variables the model contains that aren't referenced by the input/output variables listed in the spec file. In the case of En-ROADS, there were quite a few subtrees and lookups not needed for the final app, so the total code size and runtime decreased significantly with this change.

Performance

MacBook Pro (2019) | 2.4 GHz 8-core i9, 32 GB RAM, macOS 10.15

Safari:

Issue C run (ms) Wasm run (ms) Wasm init (ms) JS mem (MB) Page mem (MB)
baseline 45.8 87.5 38.0 94 685
SDE 18 46.0 85.6 18.0 39 672
SDE 19 42.8 49.4 15.0 38 25
SDE 22 34.8 44.8 15.0 38 21
SDE 23 32.7 42.8 13.0 38 32
SDE 24 26.6 38.2 13.0 39 26
SDE 7 * 24.4 34.8 10.0 39 20
En-ROADS 640 n/a 47.0 12.0 37 29
SDE 34 20.3 45.5 12.0 x x
SDE 1 14.3 29.2 10.0 36 16

Chrome:

Issue Wasm run (ms) Wasm init (ms)
SDE 24 41.6 14.1
En-ROADS 640 59.1 9.1
SDE 34 52.7 10.8
SDE 1 33.4 8.6

Firefox:

Issue Wasm run (ms) Wasm init (ms)
SDE 24 41.3 19.0
En-ROADS 640 63.7 14.0
SDE 34 56.9 14.0
SDE 1 35.8 11.0

iPhone 8 | A11, iOS 13

Issue C run (ms) Wasm run (ms) Wasm init (ms) JS mem (MB) Page mem (MB)
baseline 39.9 187.0 165.0 39 645
SDE 18 40.3 219.0 86.0 38 724
SDE 19 40.1 81.6 83.0 38 41
SDE 22 35.5 74.6 86.0 40 40
SDE 23 31.1 73.6 82.0 41 39
SDE 24 28.5 71.6 82.0 40 38
SDE 7 * 28.7 70.0 70.0 36 36
En-ROADS 640 n/a 57.1 48.0 38 39
SDE 34 22.3 50.6 60.0 x x
SDE 1 16.1 39.1 42.0 38 16

iPad Air (2013) | A7, iOS 12

Issue C run (ms) Wasm run (ms) Wasm init (ms) JS mem (MB) Page mem (MB)
baseline 151.0 1372.2 30146.0 77 331
SDE 18 166.0 1408.0 4416.0 42 395
SDE 19 151.0 837.6 1291.0 45 41
SDE 22 137.0 771.6 1484.0 44 40
SDE 23 110.1 642.2 1148.0 44 41
SDE 24 111.8 638.4 1236.0 45 37
SDE 7 * 91.7 543.8 1120.0 43 51
En-ROADS 640 n/a 210.7 570.0 41 40
SDE 34 89.9 200.7 606.0 x x
SDE 1 63.6 132.2 603.0 39 18

Size

Issue Wasm size (bytes)
baseline 1,084,036
SDE 18 773,968
SDE 19 776,851
SDE 22 737,028
SDE 23 741,668
SDE 24 741,677
SDE 7 * 616,264
En-ROADS 640 466,781
SDE 34 ** 501,731
SDE 1 289,592

Legend

Issue Date en-roads-app SDEverywhere Notes
baseline 2020/07/08 91d0918 3002ac9 baseline prior to performance work
SDE 18 2020/07/09 tbd tbd change lookup init to use static arrays
SDE 19 2020/07/09 tbd tbd break large functions into chunked subfunctions
SDE 22 2020/07/10 tbd tbd replace wrapper functions with macros
SDE 23 2020/07/10 tbd tbd replace dimension array access with simple index
SDE 24 2020/07/10 tbd tbd optimize __lookup function
SDE 7 2020/07/10 tbd tbd change from doubles to floats (* experimental, not merged)
En-ROADS 640 2020/07/20 tbd tbd use -Os instead of -O3
SDE 34 2020/09/14 tbd tbd cache input/index in __lookup function (** model upgrade to 2.7.29 with AQ work accounts for slight increase in size compared to previous numbers)
SDE 1 2020/09/14 tbd tbd eliminate unused variables in generated code

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