phetsims / resistance-in-a-wire

"Resistance in a Wire" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/resistance-in-a-wire
GNU General Public License v3.0
1 stars 4 forks source link

why has the html file size increased dramatically? #120

Closed pixelzoom closed 6 years ago

pixelzoom commented 6 years ago

Slack developer channel:

Chris Malley [4:08 PM] RIAW 1.2.8 was ~850K. 1.3.0-phetio and the recent 1.4.0 are 1.3MB. What accounts for the dramatic (53%) increase?

Jesse Greenberg [4:12 PM] Hmm, I'm not sure, I would not have expected keyboard nav to increase it by that much

pixelzoom commented 6 years ago

Assigning to @zepumph and @jessegreenberg to investigate, since they deployed 1.3.0-phetio and 1.4.0 respectively.

pixelzoom commented 6 years ago

I started to compare file sizes to see when the increase occurred, but I bailed. Odd that 1.2.9-rc.1 was deployed (by @jbphet) after 1.3.0-phetio and 1.4.0-rc.1.

pixelzoom commented 6 years ago

Looking at other sims... There appears to be a general increase in html file size. For example, function-builder 1.0.6 (1358908) to 1.0.7 (1660565), a 22% increase. I'm responsible for this sim, and there were no significant changes that I'm aware of.

jessegreenberg commented 6 years ago

For RIAW, I am seeing the largest increase between 1.2.8 at 831K (14-Apr-2017) and 1.3.0-rc.1 at 1.1M (05-May-2017).

The 1.2.9-rc.1 build is smaller at ~831K.

samreid commented 6 years ago

Looking at other sims... There appears to be a general increase in html file size. For example, function-builder 1.0.6 (1358908) to 1.0.7 (1660565), a 22% increase. I'm responsible for this sim, and there were no significant changes that I'm aware of.

images/ is 322,761 bytes after https://github.com/phetsims/function-builder/commit/1f90c8067415d297f8e304a91e753fc4c30092ed images/ is 96,026 bytes in the parent commit which is https://github.com/phetsims/function-builder/commit/20ce3711725c42e8747bcc636ca99ea225c9ae1f

This is a difference of 226735 bytes, may account for most of the added 301657 bytes (still 74922 bytes unaccounted for).

pixelzoom commented 6 years ago

@samreid said:

images/ is 322,761 bytes after phetsims/function-builder@1f90c80

Hmmm... Not sure why that commit added 226735 bytes, it looks like we were just replacing images with the same dimensions. Will investigate in https://github.com/phetsims/function-builder/issues/115.

jessegreenberg commented 6 years ago

A build of molecules-and-light off of master is 1,498,426 bytes while 1.1.15 is 1,212,717 bytes (24% increase). When I did grunt checkout-shas for molecules-and-light 1.1 and resistance-in-a-wire 1.2, I saw that those versions had no a11y in common code, so I wondered if increase was caused by a11y.

I removed all a11y code in scenery, and removed all a11y code in molecules-and-light dependencies. A build like this came down to 1,453,727 bytes, so a11y accounts for ~16% of the increase in molecules-and-light.

samreid commented 6 years ago

I recalled we decided to use the larger lodash, according to https://github.com/phetsims/sherpa/issues/58 it was only 38kb larger (minified) and we did that in early 2017. However, the dates don't match up with https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-347712176

jessegreenberg commented 6 years ago

Dates in https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-347712176 are misleading, SHAs for 1.2 are actually from 07-Mar-2016, probably more important than the date of the maintenance release.

jessegreenberg commented 6 years ago

Adding to developer meeting to make sure that the dev team knows that file size for all sims has grown by ~22% in the last 21 months, to discuss whether or not this is a concern, and to discuss next steps.

jessegreenberg commented 6 years ago

In 19-Feb-2016 example-sim was 830K and it is now 1175K, which is ~41% increase.

jessegreenberg commented 6 years ago

Dev meeting: 12/7/17 -

@jessegreenberg will look into this more thoroughly, recommendation was to look at increase to scenery and example-sim and chains since they are smaller sims.

samreid commented 6 years ago

Comparing example sim from 19-Feb-2016 built with --mangle=false and comparing to master indicated potential size increase in scenery. I tried building scenery with the Feb shas and it came out as 580KB (in the production/ dir), compared to 819KB for current master. This is a difference of 239KB. Taken together with the 38kb from lodash this accounts for 277KB of the 345KB increase (though this does not answer the question of why scenery is larger or where the other 68KB came from.)

jessegreenberg commented 6 years ago

Removing developer meeting label, was discussed last week and plan is to continue with https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-350079994

jessegreenberg commented 6 years ago

I was unable to get example-sim to build off of old SHAs, so I have been testing with builds of RIAW and scenery.

A build of RIAW master is 1.24 MB. A build of RIAW 1.2 is 830 kb. This is an increase of 410 kb. I tried inspecting a diff of old and new builds with --mangle=false, but there are too many changes to isolate any one increase. Here is a list of some additions I found in the diff:

I tried building before/after removing some of the "Large" features and comparing build sizes to get a sense of their impact: A11y: 53 KB RichText: 14 KB AlignBox/AlignGroup: 7 KB Picker - 7 KB Constructive Area Geometry: 25 KB

We have established that lodash upgrade added 38KB. The other new preloads look small, maybe 1-10KB. The new images are ~1KB each.

So this would explain about ~155 KB of the 410 KB increase in RIAW.

I am not sure how to gauge the impact of PhET-iO. The "misc" additions probably add up to something significant.

jessegreenberg commented 6 years ago

Taken together with the 38kb from lodash this accounts for 277KB of the 345KB increase (though this does not answer the question of why scenery is larger or where the other 68KB came from.)

This and https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-353200809 make it seem possible that the increase in file size could be caused by code additions. In scenery, I see many new types that have been added in the past year that are large like everything under scenery/js/display/drawables/ and new listener types under scenery/js/listeners/. All of these could explain the increase in scenery build size that @samreid observed.

jessegreenberg commented 6 years ago

I wrote a script that checks out the SHA of each scenery commit in the last year, builds, and prints size if build succeeds. This plot shows build after commits from Wed Mar 9 23:47:51 2016 to Mon Dec 18 17:03:08 2017. 0s are build failures. Growth seems fairly steady.

All builds failed after Tue Dec 13 12:14:24 2016 since dependencies stayed on master. I disabled eslint to get more builds. Between Dec 13 2016 and Dec 20 2017, scenery grew from 716037 bytes to 821620 bytes, about 105 KB.

mem plot

pixelzoom commented 6 years ago

During today's status meeting, @jessegreenberg said he wanted to discuss this at today's developer meeting, so labeling for discussion.

pixelzoom commented 6 years ago

12/21/17 dev meeting:

@jessegreenberg will do something similar to https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-353208750 for RIAW. Determine if it's a slow steady growth, or if there are spikes.

If it's slow steady growth, @jonathanolson mentioned that there is low-hanging fruit to be investigated for reducing file size.

@pixelzoom mentioned that spikes might be due to common-code that has introduced a lot of dependencies that are not needed by a sim.

jessegreenberg commented 6 years ago

https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-353428099

I will look into checking out dependencies at the same date as eac RIAW SHA to get a better sense of project growth.

samreid commented 6 years ago

I read about git checkout by date here: https://stackoverflow.com/questions/6990484/git-checkout-by-date

jessegreenberg commented 6 years ago

I modified the script to get scenery commits in the last 2 years, and for each commit, check out sim and dependencies at commit date, npm prune and update in chipper and sim, and print size of built sim file. Here is a plot of the output after doing this. 2-year

Here is a zoom in on 2017, it looks like the biggest jumps are in Feb and Sep of 2017. 1-year

jessegreenberg commented 6 years ago

February jump happened between these two commits:

946463 bytes after

commit 1e6b9c04bfff3c3f4b9161c8090b6e6d455bde2a
Author: samreid <reids@colorado.edu>
Date:   Fri Feb 3 15:55:20 2017 -0700

    Updated to lodash 4.17.4, see https://github.com/phetsims/sherpa/issues/58

1035382 bytes after:

commit ce49c33ff5c41a7dd26133aa4287ca2d5c761ba8
Merge: 1e6b9c04 ecc12dea
Author: samreid <reids@colorado.edu>
Date:   Mon Feb 6 09:42:58 2017 -0700

    Merge branch 'tandem'
jessegreenberg commented 6 years ago

The jump in September happened sometime between these commits: 1168104 bytes after:

commit aaa3ccacff30fff595568ef04b2ac41fc6f83523
Author: Jesse Greenberg <jesse.greenberg@colorado.edu>
Date:   Thu Sep 21 17:39:02 2017 -0600

    sort AccessibleInstances under trail's leaf node if it exists, otherwise sort under closest ancestor AccessibleInstance, see #674

1213422 bytes after

commit fd3efe94a90388d3dca0a02007f005262f9f21ab
Author: zepumph <michael.kauzmann@colorado.edu>
Date:   Thu Sep 28 15:51:19 2017 -0800

    Use phetioEvents directly instead of started/ended emitters in SimpleDragHandler, see phetsims/phet-io#1223
zepumph commented 6 years ago

Unassigning before my time off.

jessegreenberg commented 6 years ago

@samreid do you think that "Merge branch 'tandem'" could be the cause of the spike in Feb 2017?

git log --since "2017-09-21" --until "2017-09-28" in kite shows 30 commits related to CAG, presumably the increase in size in that week is mostly from those changes? I will try another test against those commits, it would show us if growth is steady during that time.

jessegreenberg commented 6 years ago

Results are in: kite-plot

The commit on the jump on Sep 26 is https://github.com/phetsims/kite/commit/d1fafef7e6fff41748c793751e6d26bbf3f77ad8, which is where Graph was added to Shape.

The commit on the size increase on Sep 28 seems unlikely to have caused it. https://github.com/phetsims/kite/commit/7a8ffcab1aef0b330083fb10c8222f1301024495

jessegreenberg commented 6 years ago

RIAW got a KeyboardHelpDialog on Sep 27, 2017 which included the first usages of AlignGroup and KeyNode in this sim, and the view code required for layout. I would be surprised if this caused the entire 20KB increase between Sep 27 and 28.

Looking through commits around september 28 with git log --since "2017-09-27" --until "2017-09-29" I see lots of commits throughout the project with a message like "Use phetioEvents instead of started/ended emitters". A new file called phetioEvents was added at this time, but it also looks fairly small.

Adding developer meeting label to go over comments since https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-353761656 and to discuss next steps.

samreid commented 6 years ago

@samreid do you think that "Merge branch 'tandem'" could be the cause of the spike in Feb 2017?

Merge branch tandem has 2 parents:

The former is known to add many KB. I skimmed through file changes in the latter and it looks like it is adding 100+ lines of code (probably between 100 and 300).

I wonder if we should be asking the question "how can we reduce our download size"--that is, focusing on being able to visualize the size of different components in a sim build to see if anything looks like it needs to be reduced/optimized?

jessegreenberg commented 6 years ago

Discussion from 1/4/2018:

Regarding the big jump in size in early February, we would like to learn more. We will make a similar plot over commits across the whole project in that time frame.

Regarding

I wonder if we should be asking the question "how can we reduce our download size"

@ariel-phet says that right now that is not necessary, the goal for this issue was just to determine what caused the increase. Even with the observed increase our sim sizes are acceptable.

jessegreenberg commented 6 years ago

We are also going to put this script in perennial.

jessegreenberg commented 6 years ago

A plot against commits for the entire project around the increase in february looks like this: all-plot

Zooming in on Feb 3: feb3-plot

All commits at the spike have the message "Updated to lodash 4.17.4"

So this is caused by the lodash update. Why didn't it show up in https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-353761862?

jessegreenberg commented 6 years ago

The last commit that had the sim < 950 KB was for scenery at Fri Feb 3, 207 15:55:20 with message "Updated to lodash 4.17.4..."

One second later at Fri Feb 3, 2017 15:55:21 sherpa got a commit with message "Updated to lodash 4.17.4..."

Which caused the increase of 88.2 KB.

jessegreenberg commented 6 years ago

Adding dev meeting label to go over https://github.com/phetsims/resistance-in-a-wire/issues/120#issuecomment-355615214 and to see if we should talk about this more or begin to think about optimizing file size.

jessegreenberg commented 6 years ago

Discussed in dev meeting:

For now, we don't need to worry about optimizing file size. This issue can be closed. For now we should be watching file size. Every maintenance/minor release we should be watching file sizes to make sure that our sims aren't growing out of control.

This issue can be closed.