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

(refactor): upgrade Meteor to 1.6.1 #3615

Merged
merged 12 commits into from Feb 15, 2018

Conversation

7 participants
@spencern
Copy link
Member

spencern commented Jan 31, 2018

This PR upgrades Reaction to Meteor 1.6.1

See the Meteor 1.6.1 announcement or release notes for more details about the underlying changes to Meteor.

The biggest change is that we're upgrading to Babel 7.

"@babel/runtime": "7.0.0-beta.38",

and in our dev-dependencies

"@babel/cli": "^7.0.0-beta.38",
"@babel/core": "^7.0.0-beta.38",
"@babel/preset-react": "^7.0.0-beta.38",
babel-preset-meteor: 7.0.0-beta.38

Our babel presets now looks like this:

"presets": [
  "meteor",
  "@babel/preset-react"
]

Yes, we've removed stage-2 and env from our presets. That's recommended as meteor now includes babel-preset-meteor

Upgrading meteor also resulted in this automatic output.

Changes to your project's package version selections from updating the release:

accounts-base              upgraded from 1.4.0 to 1.4.2
accounts-facebook          upgraded from 1.3.0 to 1.3.1
accounts-google            upgraded from 1.3.0 to 1.3.1
accounts-twitter           upgraded from 1.4.0 to 1.4.1
autoupdate                 upgraded from 1.3.12 to 1.4.0
babel-compiler*            upgraded from 6.24.7 to 7.0.0
babel-runtime              upgraded from 1.1.1 to 1.2.0
boilerplate-generator      upgraded from 1.3.1 to 1.4.0
callback-hook              upgraded from 1.0.10 to 1.1.0
check                      upgraded from 1.2.5 to 1.3.0
coffeescript               downgraded from 1.12.7_3 to 1.0.17
coffeescript-compiler      removed from your project
ddp-client                 upgraded from 2.2.0 to 2.3.1
ddp-common                 upgraded from 1.3.0 to 1.4.0
ddp-server                 upgraded from 2.1.1 to 2.1.2
diff-sequence              upgraded from 1.0.7 to 1.1.0
dynamic-import             upgraded from 0.2.1 to 0.3.0
ecmascript                 upgraded from 0.9.0 to 0.10.0
ecmascript-runtime-client  upgraded from 0.5.0 to 0.6.0
es5-shim                   upgraded from 4.6.15 to 4.7.0
http                       upgraded from 1.3.0 to 1.4.0
id-map                     upgraded from 1.0.9 to 1.1.0
meteor-base                upgraded from 1.2.0 to 1.3.0
minifier-css               upgraded from 1.2.16 to 1.3.0
minifier-js                upgraded from 2.2.2 to 2.3.1
modules                    upgraded from 0.11.1 to 0.11.3
mongo                      upgraded from 1.3.1 to 1.4.2
oauth                      upgraded from 1.2.0 to 1.2.1
ordered-dict               upgraded from 1.0.9 to 1.1.0
promise                    upgraded from 0.10.0 to 0.10.1
random                     upgraded from 1.0.10 to 1.1.0
reload                     upgraded from 1.1.11 to 1.2.0
retry                      upgraded from 1.0.9 to 1.1.0
server-render              added, version 0.3.0
shim-common                added, version 0.1.0
socket-stream-client       added, version 0.1.0
standard-minifier-js       upgraded from 2.2.3 to 2.3.1
url                        upgraded from 1.1.0 to 1.2.0
webapp                     upgraded from 1.4.0 to 1.5.0

* These packages have been updated to new versions that are not backwards
compatible.
reaction: updated to Meteor 1.6.1.

Changes to your project's package version selections from updating package versions:

babel-runtime        upgraded from 1.2.0 to 1.2.2
caching-compiler     upgraded from 1.1.9 to 1.1.11
cfs:storage-adapter  upgraded from 0.2.3 to 0.2.4
es5-shim             upgraded from 4.7.0 to 4.7.3
jquery               upgraded from 1.11.10 to 1.11.11
less                 upgraded from 2.7.11 to 2.7.12
modules-runtime      upgraded from 0.9.1 to 0.9.2
mongo                upgraded from 1.4.2 to 1.4.3
npm-mongo            upgraded from 2.2.33 to 2.2.34


The following top-level dependencies were not updated to the very latest version available:
 * aldeed:autoform 5.8.1 (6.3.0 is available)
 * aldeed:collection2 2.10.0 (3.0.0 is available)
 * aldeed:schema-index 1.1.1 (3.0.0 is available)
 * percolate:migrations 0.9.8 (1.0.2 is available)
 * vsivsi:job-collection 1.4.0 (1.5.2 is available)

Newer versions of the following indirect dependencies are available:
 * aldeed:collection2-core 1.2.0 (2.1.2 is available)
 * aldeed:schema-deny 1.1.0 (3.0.0 is available)
 * coffeescript 1.0.17 (2.0.3_4 is available)
 * momentjs:moment 2.19.4 (2.20.1 is available)
These versions may not be compatible with your project.
To update one or more of these packages to their latest
compatible versions, pass their names to `meteor update`,
or just run `meteor update --all-packages`.

I'm seeing tests pass locally running reaction test.

I'm not 100% certain that I've upgraded everything that needs to be upgraded, but I'm not sure what else might need to get checked.

I'd like to do a more full run through the app before taking off the WIP.

I've tested adding a basic product to the cart and checking out, but nothing more in depth at this point.

A few ugly things:

  • Occasionally the app hangs while building during startup - haven't figured out why or how to reliably reproduce it, but it's happened a few times.
  • the meteor update script downgraded coffeescript and removed the coffeescript compiler from the project. How many coffeescript packages do we have left? Are we close to being able to remove that dependency entirely? The only one I know of off the top of my head is vsivsi:job-collection which @zenweasel is working on now.
  • While (literally) working on this PR, babel released 7.0.0-beta.39 - the babel-meteor-preset has not updated yet and contains a reference to a plugin that does not exist in beta-39 for now I've removed the caret as it's been installing incompatible versions. This should be revisited when meteor bumps the versions in babel-meteor-preset
@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Jan 31, 2018

One thing I have seen in the forums is a couple of people reporting that collection hooks don't work after upgrading but def not for everyone. Just something to double-check when testing.

@spencern spencern requested review from aldeed and aaronjudd Jan 31, 2018

@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Jan 31, 2018

We may want to switch from dispatch:mocha to meteortesting:mocha (same package, now community maintained) as recommended in the release doc.

@aaronjudd

This comment has been minimized.

Copy link
Contributor

aaronjudd commented Jan 31, 2018

@zenweasel agreed - I think @aldeed may have mentioned this as well.

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Jan 31, 2018

@zenweasel @aaronjudd
I've replaced dispatch:mocha with meteortesting:mocha in the most recent commit.

@abernix

This comment has been minimized.

Copy link

abernix commented Jan 31, 2018

I won't definitively claim that Meteor isn't creating some problems for you here (we're trying to investigate this further to see if it's anything we can/could-have avoided), but I'd highly recommend moving from practicalmeteor:chai and practicalmeteor:sinon to the direct npm options for each, respectively.

Those (unmaintained) packages have some aging dependencies. This may be contributing to a very low coffeescript version constraint (the version of coffeescript by Meteor "0.9.3" is quite low), which could be contributing to the problems reported above, not to mention you're missing out on most of the new features of each!

I've witnessed this in some other projects and, in most cases, those packages are the lone dependents on coffeescript: A meteor list --tree within this project would be able to shine more light on that though; I haven't checked this project specifically. Migrating them to npm was relatively simple, as many of the APIs are still fairly close (though the version gap is widening!).

@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Jan 31, 2018

The practicalmeteor packages offer some significant advantages over the npm versions (basically allows you to do expectations on toHaveBeenCalled so it would require rewriting a lot of tests. Or maybe there's some way to bring those packages into the project? I looked at this about a year ago and it didn't make sense but it's probably time to figure out a real solution.

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Jan 31, 2018

@abernix thank you for the suggestions. Looking at the tree, it appears that you're correct.

practicalmeteor:chai@2.1.0_1
└─┬ coffeescript@1.0.17
  ├── caching-compiler@1.1.11 (expanded above)
  └── ecmascript@0.10.0 (top level)
practicalmeteor:sinon@1.14.1_2
├── coffeescript@1.0.17 (expanded above)
└── practicalmeteor:chai@2.1.0_1 (top level)
...
vsivsi:job-collection@1.4.0
├── check@1.3.0 (top level)
├── coffeescript@1.0.17 (expanded above)
├── mongo@1.4.3 (top level)
└── mrt:later@1.6.1

Full meteor list --tree output
https://gist.github.com/spencern/b34773e160633c6a5c48d8fc9b70130a

@abernix

This comment has been minimized.

Copy link

abernix commented Jan 31, 2018

@zenweasel I'm not super close to all these projects, but both of those packages I mentioned are relatively thin wrappers around the old, similarly named npm modules. I don't believe they implement much additional functionality, as you can see in their repositories (Sinon, Chai).

The "spies" notation you're using (toHaveBeenCalled) looks like Jest/Jasmine to me, but I think Sinon has something along the lines of .to.have.been.called.with, and I think Chai supports a Sinon plugin to do something similar.

@aaronjudd
Copy link
Contributor

aaronjudd left a comment

did not test jobs as @zenweasel mentioned, but saw no obvious breaking changes

@aldeed

This comment has been minimized.

Copy link
Member

aldeed commented Jan 31, 2018

Rewriting tests would mostly just be some simple search and replace on the matchers, and might be worth it. If I'm not using Jest, I typically use the expect NPM package, which is what Jest uses behind the scenes. Would just need to replace the imports with import expect from "expect" and search/replace any matchers that have different names

@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Jan 31, 2018

I think rewriting the tests and getting rid of the Meteor dependencies is something we should get on the board and start on if it's going to stick us with Coffeescript for just those packages.

@@ -75,7 +75,7 @@ gadicc:blaze-react-component

# Testing packages
dburles:factory
dispatch:mocha
meteortesting:mocha

This comment has been minimized.

Copy link
@aldeed

aldeed Jan 31, 2018

Member

@spencern I think I read that the test driver package no longer needs to be listed here. So I would remove it from here and just make sure the CLI reaction test command is updated. My real preference would be:

  1. Change the npm "test" script to run the app tests. (Copy the command the CLI currently uses.)
  2. Change the CLI reaction test command to simply run npm test
@hrath2015

This comment has been minimized.

Copy link
Collaborator

hrath2015 commented Feb 1, 2018

On windows getting the below error... It was completely gone in 1.6 and back in 1.6.1. Will build couple more times and see the results.

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory 1: node_module_register 2: v8::internal::FatalProcessOutOfMemory 3: v8::internal::FatalProcessOutOfMemory 4: v8::internal::Factory::NewTransitionArray 5: v8::internal::EhFrameIterator::DecodeSLeb128 6: v8::internal::JSReceiver::GetOwnPropertyDescriptor 7: v8::internal::JSReceiver::GetOwnPropertyDescriptor 8: v8::internal::JSReceiver::GetOwnPropertyDescriptor 9: v8::internal::JSReceiver::class_name 10: v8::internal::JSReceiver::GetOwnPropertyDescriptor 11: v8::internal::LookupIterator::PrepareTransitionToDataProperty 12: std::vector<v8::internal::compiler::MoveOperands * __ptr64,v8::internal::ZoneAllocator<v8::internal::compiler::MoveOperands * __ptr64> >::_Reallocate 13: std::vector<v8::internal::compiler::MoveOperands * __ptr64,v8::internal::ZoneAllocator<v8::internal::compiler::MoveOperands * __ptr64> >::_Reallocate 14: std::vector<v8::internal::compiler::MoveOperands * __ptr64,v8::internal::ZoneAllocator<v8::internal::compiler::MoveOperands * __ptr64> >::_Reallocate 15: std::vector<v8::internal::compiler::MoveOperands * __ptr64,v8::internal::ZoneAllocator<v8::internal::compiler::MoveOperands * __ptr64> >::_Reallocate 16: std::vector<v8::internal::compiler::MoveOperands * __ptr64,v8::internal::ZoneAllocator<v8::internal::compiler::MoveOperands * __ptr64> >::_Reallocate 17: 00000200E1A847A1

@hrath2015

This comment has been minimized.

Copy link
Collaborator

hrath2015 commented Feb 1, 2018

It is failing after taking more than 25 mins to build and link on windows 10. 😭

@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Feb 1, 2018

I saw the same error as @hrath2015 and long build times on my OSX machine but just chocked it up to environment weirdness

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 2, 2018

@hrath2015 @zenweasel are you seeing this on every build for windows or just occasionally?

@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Feb 5, 2018

I'm able to build fine on Windows 10. I did get some strange errors when doing a reaction init but I don't know if any of those are related.

@hrath2015

This comment has been minimized.

Copy link
Collaborator

hrath2015 commented Feb 5, 2018

Tried it all over again. Restarted machine and pulled the latest. It is same. After 20 mins it failed with all sorts of messages.

@hrath2015

This comment has been minimized.

Copy link
Collaborator

hrath2015 commented Feb 5, 2018

even set TOOL_NODE_FLAGS="--max-old-space-size=1024" did not help

@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Feb 5, 2018

@hrath2015 I was able to build the first couple of times, but now when I start I see the same/similar error

@zenweasel

This comment has been minimized.

Copy link
Member

zenweasel commented Feb 5, 2018

Looks possibly similar to this issue: meteor/meteor#9568

@hrath2015

This comment has been minimized.

Copy link
Collaborator

hrath2015 commented Feb 5, 2018

Looks like we are back to situation when meteor 1.5 was released. Build Failing most of the time on windows. When 1.6 was released, error CALL_AND_RETRY_LAST Allocation was completing gone and build times were faster. I did testing then (in Nov 17) as below and saw consistent success over a period of time.

image

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 5, 2018

@hrath2015 @zenweasel do you believe this to be an issue with our upgrade to Meteor 1.6.1 or an issue with the 1.6.1 release itself? It seems like there are some substantial problems building 1.6.1 on Windows, but as @hrath2015 mentions, we’ve seen that before and it sounds like there may be a work around.

@spencern spencern changed the base branch from master to release-1.8.0 Feb 13, 2018

spencern added some commits Feb 13, 2018

Merge remote-tracking branch 'origin/release-1.8.0' into refactor-360…
…0-spencern-upgrade-meteor-1.6.1

# Conflicts:
#	package-lock.json
(chore): pin babel 7 versions to `7.0.0-beta.38`
The fails to start when upgrading to `babel-preset-meteor: 7.0.0-beta.40-1`
@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 13, 2018

Pinned babel versions to 7.0.0-beta.38 because the app fails to start when using the 7.0.0-beta.40 which was released today. Issue created in the babel-plugin-meteor repo.

meteor/babel-preset-meteor#8

@spencern spencern changed the title [WIP] Refactor 3600 spencern upgrade meteor 1.6.1 (refactor): upgrade meteor to 1.6.1 Feb 14, 2018

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 14, 2018

Both @kieckhafer and I are seeing long initial build times the first time the app starts
Profiling and following these steps:

  1. reaction init release-1.6.1
  2. cd release-1.6.1
  3. git checkout refactor-3600-spencern-upgrade-meteor-1.6.1
  4. METEOR_PROFILE=1 reaction

Initial build

(#3) Total: 818,212 ms (Build App)

That's 13 minutes to build the app, we haven't seen times like that in 1.6

Of those 13 minutes, about 80% of it is on the server, and ~2.5 minutes are on the client

On the client:

|    ├─ bundler.bundle..makeClientTarget......................162,838 ms (1)
|    │     ├─ Target#_runCompilerPlugins......................112,782 ms (1)
|    │     ├─ Target#_emitResources............................46,194 ms (1)

While on the server:

|    ├─ bundler.bundle..makeServerTarget......................606,498 ms (1)
|    │     ├─ Target#_runCompilerPlugins......................482,170 ms (1)
|    │     │  ├─ plugin ecmascript............................481,174 ms (1)
|    │     │  │  ├─ optimistic statOrNull......................80,791 ms (141903)
|    │     │  │  │  ├─ files.stat                              16,958 ms (22851)
|    │     │  │  │  ├─ safeWatcher.watch.......................11,940 ms (10628)
|    │     │  │  │  │  ├─ files.stat                            9,012 ms (8668)
|    │     │  │  │  │  └─ other safeWatcher.watch               2,928 ms
|    │     │  │  │  ├─ files.lstat                              8,234 ms (7820)
|    │     │  │  │  └─ other optimistic statOrNull             43,660 ms
|    │     │  │  ├─ Babel.compile                             378,293 ms (707)

Once we get to server startup, everything is mostly reasonable except for oauth1

| Server startup...............................................80,294 ms (1)
| ├─ Load server bundles.......................................70,862 ms (1)
| │  ├─ packages/oauth1.js                                     65,377 ms (1)

Full output https://gist.github.com/spencern/729361e5fa84582094597060fdd54c56

Second and subsequent builds

On the second build and subsequent builds after that, the app takes a little over a minute to build and start.

| (#3) Total: 58,619 ms (Build App)

The proportional amount of time it takes to make the client and make the server have flipped.

|    ├─ bundler.bundle..makeClientTarget.......................36,146 ms (1)
|    ├─ bundler.bundle..makeServerTarget.......................12,843 ms (1)

And the oauth package is not an issue

| Server startup................................................7,025 ms (1)
| ├─ Load server bundles........................................3,806 ms (1)
| │  ├─ packages/oauth1.js                                          1 ms (1)

Here's the full output from the second build: https://gist.github.com/spencern/3a19e69105b4aaaf39a303b452b11949

We are using a pinned version of babel-meteor-plugin (7.0.0-beta.38-1) because beta-40 would cause the app not to start.

At this point it looks like the culprit is likely babel, our specific mixture of babel plugins, or a misconfigured package.json. I'm not really sure which of those is most likely.

Performance Issues

@spencern spencern changed the title (refactor): upgrade meteor to 1.6.1 (refactor): upgrade Meteor to 1.6.1 Feb 14, 2018

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 14, 2018

To add some context to the above performance profiling:

on our release-1.8.0 branch which this is built on top of and is currently on the 1.6.0 branch:

Initial build times:

| (#3) Total: 134,402 ms (Build App)
|    ├─ bundler.bundle..makeClientTarget.......................85,494 ms (1)
|    ├─ bundler.bundle..makeServerTarget.......................41,371 ms (1)
| (#1) Total: 18,925 ms (Server startup)
| │  ├─ packages/oauth1.js                                      4,351 ms (1)
@aldeed

This comment has been minimized.

Copy link
Member

aldeed commented Feb 14, 2018

@spencern Maybe try pinning back to ecmascript@0.9.0 in Meteor packages? That package has had a few of its compiler/babel plugins change between those releases. (In addition to the one you already pinned.) https://github.com/meteor/meteor/commits/devel/packages/ecmascript/package.js

If that doesn't help, then it could be some issue in the build tool itself I suppose.

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 14, 2018

@aldeed Unfortunately it seems that Meteor v1.6.1 depends on ecmascript@0.10.0 or later. I saw that it had bumped to 0.10.0 when I upgraded, but attempting to pin back to 0.9.0 confirms that.

   * ecmascript@=0.9.0 <- top level
   * ecmascript@~0.10.0 <- top level
   * ecmascript@0.10.0 <- webapp 1.5.0 <- accounts-oauth 1.1.15 <- accounts-facebook 1.3.1
   * ecmascript@0.10.0 <- webapp 1.5.0 <- meteor-base 1.3.0
   * ecmascript@0.9.0 <- ejson 1.1.0
   * ecmascript@0.10.0 <- boilerplate-generator 1.4.0 <- webapp 1.5.0 <- accounts-oauth 1.1.15 <- accounts-facebook 1.3.1
   * ecmascript@0.10.0 <- boilerplate-generator 1.4.0 <- webapp 1.5.0 <- meteor-base 1.3.0
   * ecmascript@0.4.3 <- webapp-hashing 1.0.9 <- webapp 1.5.0 <- accounts-oauth 1.1.15 <- accounts-facebook 1.3.1
   * ecmascript@0.4.3 <- webapp-hashing 1.0.9 <- webapp 1.5.0 <- meteor-base 1.3.0
   * ecmascript@0.10.0 <- ddp-client 2.3.1 <- ddp 1.4.0 <- accounts-base 1.4.2
   * ecmascript@0.10.0 <- check 1.3.0
   * ecmascript@0.10.0 <- random 1.1.0
   * ecmascript@0.10.0 <- retry 1.1.0 <- autoupdate 1.4.0 <- hot-code-push 1.0.4 <- meteor-base 1.3.0
   * ecmascript@0.10.0 <- id-map 1.1.0 <- binary-heap 1.0.10 <- mongo 1.4.2
   * ecmascript@0.10.0 <- callback-hook 1.1.0 <- accounts-base 1.4.2
   * ecmascript@0.10.0 <- ddp-common 1.4.0 <- ddp-client 2.3.1 <- ddp 1.4.0 <- accounts-base 1.4.2
   * ecmascript@0.10.0 <- reload 1.2.0
   * ecmascript@0.10.0 <- socket-stream-client 0.1.0 <- ddp-client 2.3.1 <- ddp 1.4.0 <- accounts-base 1.4.2
   * ecmascript@0.10.0 <- diff-sequence 1.1.0 <- ddp-client 2.3.1 <- ddp 1.4.0 <- accounts-base 1.4.2
   * ecmascript@0.10.0 <- diff-sequence 1.1.0 <- minimongo 1.4.3 <- aldeed:schema-index 1.1.0
   * ecmascript@0.10.0 <- diff-sequence 1.1.0 <- mongo 1.4.2
   * ecmascript@0.10.0 <- ddp-server 2.1.2 <- ddp 1.4.0 <- accounts-base 1.4.2
   * ecmascript@0.9.0 <- minimongo 1.4.3 <- aldeed:collection2-core 1.0.0 <- aldeed:schema-index 1.1.0
   * ecmascript@0.9.0 <- minimongo 1.4.3 <- aldeed:schema-index 1.1.0
   * ecmascript@0.10.0 <- ordered-dict 1.1.0 <- blaze 2.3.2 <- accounts-base 1.4.2
   * ecmascript@0.10.0 <- server-render 0.3.0 <- es5-shim 4.7.2
   * ecmascript@0.10.0 <- shim-common 0.1.0 <- es5-shim 4.7.2
   * ecmascript@0.10.0 <- autoupdate 1.4.0 <- hot-code-push 1.0.4 <- meteor-base 1.3.0
   * ecmascript@0.10.0 <- mongo 1.4.2
   * ecmascript@0.9.0 <- allow-deny 1.1.0 <- mongo 1.4.2
   * ecmascript@0.5.7 <- templating 1.2.13 <- aldeed:template-extension 0.1.0
   * ecmascript@0.1.3-rc.0 <- caching-html-compiler 1.0.1-rc.0 <- templating 1.2.13 <- aldeed:template-extension 0.1.0
   * ecmascript@0.5.9 <- caching-compiler 1.1.9 <- caching-html-compiler 1.0.1-rc.0 <- templating 1.2.13 <- aldeed:template-extension 0.1.0
   * ecmascript@0.5.9 <- caching-compiler 1.1.9 <- less 2.7.11
   * ecmascript@0.4.3-rc.6 <- templating-tools 1.0.4-rc.6 <- caching-html-compiler 1.0.1-rc.0 <- templating 1.2.13 <- aldeed:template-extension 0.1.0
   * ecmascript@0.4.3-rc.6 <- templating-tools 1.0.4-rc.6 <- templating 1.2.13 <- aldeed:template-extension 0.1.0
   * ecmascript@0.5.9 <- juliancwirko:postcss 1.2.0
   * ecmascript@0.10.0 <- minifier-css 1.3.0 <- juliancwirko:postcss 1.2.0
   * ecmascript@0.9.0 <- reactive-dict 1.2.0
   * ecmascript@0.10.0 <- rate-limit 1.0.9 <- ddp-rate-limiter 1.0.7
   * ecmascript@0.9.0 <- less 2.7.11
   * ecmascript@0.9.0 <- accounts-base 1.4.2
   * ecmascript@0.1.6 <- mdg:validated-method 0.2.0
   * ecmascript@0.9.0 <- shell-server 0.3.1
   * ecmascript@0.10.0 <- standard-minifier-js 2.3.1
   * ecmascript@0.9.0 <- accounts-password 1.5.0
   * ecmascript@0.9.0 <- accounts-facebook 1.3.1
   * ecmascript@0.9.0 <- accounts-google 1.3.1
   * ecmascript@0.7.3 <- google-oauth 1.2.5 <- accounts-google 1.3.1
   * ecmascript@0.9.0 <- accounts-twitter 1.4.1
   * ecmascript@0.1.6 <- mdg:validation-error 0.3.0 <- aldeed:simple-schema 1.5.3 <- aldeed:collection2-core 1.0.0 <- aldeed:schema-index 1.1.0
   * ecmascript@0.4.0-rc.0 || 0.4.0-beta.10 || 0.2.1-modules.0 <- gadicc:blaze-react-component 1.0.0
   * ecmascript@0.3.0 <- meteortesting:mocha 0.4.2
   * ecmascript@0.4.1 <- practicalmeteor:mocha-core 1.0.1 <- meteortesting:mocha 0.4.2
   * ecmascript@0.3.0 <- meteortesting:browser-tests 0.1.1 <- meteortesting:mocha 0.4.2
   * ecmascript@0.1.5 <- johanbrook:publication-collector 1.0.0

I'm going to see if I can get the app to start with the v1.6.2 beta

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 14, 2018

Ran meteor update to bump any base packages up and looks like both babel-compiler and ecmascript were on outdated versions.

babel-compiler                    upgraded from 7.0.0 to 7.0.4
base64                            upgraded from 1.0.10 to 1.0.11
ecmascript                        upgraded from 0.10.0 to 0.10.4
ecmascript-runtime-client         upgraded from 0.6.0 to 0.6.2
johanbrook:publication-collector  upgraded from 1.0.10 to 1.1.0
minifier-css                      upgraded from 1.3.0 to 1.3.1
minifier-js                       upgraded from 2.3.1 to 2.3.2
modules                           upgraded from 0.11.3 to 0.11.4
promise                           upgraded from 0.10.1 to 0.10.2
rate-limit                        upgraded from 1.0.8 to 1.0.9
standard-minifier-js              upgraded from 2.3.1 to 2.3.2

Cut about 50% off the startup time. Still not fast, but might not be a blocker any more as it still seems to only be on the first load.

| (#3) Total: 387,275 ms (Build App)
| (#1) Total: 72,622 ms (Server startup)

Profile: https://gist.github.com/spencern/04649679a2b446d2ab6019b5193d514e

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 14, 2018

Attempted to upgrade to the latest beta release METEOR@1.6.2-beta.7 and the app will not build. Seeing FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory

<--- Last few GCs --->

[16298:0x103800600]  1303774 ms: Mark-sweep 1419.6 (2127.9) -> 1419.3 (2127.9) MB, 1273.4 / 0.4 ms  allocation failure GC in old space requested
[16298:0x103800600]  1305003 ms: Mark-sweep 1419.3 (2127.9) -> 1419.3 (2075.4) MB, 1228.3 / 0.4 ms  last resort GC in old space requested
[16298:0x103800600]  1306212 ms: Mark-sweep 1419.3 (2075.4) -> 1419.7 (2058.9) MB, 1208.9 / 0.4 ms  last resort GC in old space requested


<--- JS stacktrace --->

==== JS stack trace =========================================

Security context: 0xcbf252a5ee1 <JSObject>
    1: /* anonymous */ [/Users/spencer/.meteor/packages/meteor-tool/.1.6.2-beta.7.10jdi0.5gnt2d++os.osx.x86_64+web.browser+web.browser.legacy+web.cordova/mt-os.osx.x86_64/dev_bundle/lib/node_modules/source-map/lib/source-node.js:~336] [pc=0x11f82bf9d568](this=0xcbf68f0c0e9 <JSGlobal Object>,chunk=0xcbf368384a1 <String[11]: "./ru.json">,original=0xcbf7ef3d611 <Object map = 0xcbffe17c859>)
    2: S...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
{ Error: Command failed: meteor --settings settings/dev.settings.json --raw-logs
    at checkExecSyncError (child_process.js:601:13)
    at execSync (child_process.js:641:13)
    at _callee$ (/usr/local/lib/node_modules/reaction-cli/dist/commands/run.js:82:43)
    at tryCatch (/usr/local/lib/node_modules/reaction-cli/node_modules/regenerator-runtime/runtime.js:65:40)
    at Generator.invoke [as _invoke] (/usr/local/lib/node_modules/reaction-cli/node_modules/regenerator-runtime/runtime.js:303:22)
    at Generator.prototype.(anonymous function) [as next] (/usr/local/lib/node_modules/reaction-cli/node_modules/regenerator-runtime/runtime.js:117:21)
    at step (/usr/local/lib/node_modules/reaction-cli/dist/commands/run.js:108:191)
    at /usr/local/lib/node_modules/reaction-cli/dist/commands/run.js:108:361
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

For now I think 1.6.1 or 1.6.0 is the best we've got. Upgrading to this current commit will put us on Babel 7.

@spencern

This comment has been minimized.

Copy link
Member Author

spencern commented Feb 14, 2018

Note that this comment is about upgrading to Meteor 1.6.2 beta.7 which is out of the scope of this ticket and I'll be moving to another PR / Issue

Starting with TOOL_NODE_FLAGS="--max-old-space-size=4096" enables the app to build.

Ran into this error after the initial build had completed, but setting the max-old-space-size seems to pacify the memory error

TypeError: Cannot read property 'Dependency' of undefined
    at packages/aldeed_simple-schema.js:1542:45
    at packages/aldeed_simple-schema.js:2160:4
    at packages/aldeed_simple-schema.js:3128:3
    at profileWrapper (/Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/profile.js:288:16)
    at /Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/boot.js:411:36
    at Array.forEach (<anonymous>)
    at /Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/boot.js:220:19
    at profileWrapper (/Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/profile.js:288:16)
    at /Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/boot.js:471:5
    at profileWrapper (/Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/profile.js:288:16)
    at time (/Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/profile.js:309:28)
    at Function.run (/Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/profile.js:522:14)
    at /Users/spencer/reaction/release-1.6.1/.meteor/local/build/programs/server/boot.js:470:11
=> Exited with code: 1

@kieckhafer kieckhafer self-requested a review Feb 15, 2018

@kieckhafer
Copy link
Member

kieckhafer left a comment

Can confirm that build time was about 50% of what it was yesterday.

~7 min for the initial build, ~45s for subsequent builds.

@aaronjudd
Copy link
Contributor

aaronjudd left a comment

With the latest updates, performance when not using the profiler seems roughly on par with 1.6, and I'm not seeing any blockers. Profiler shows a variety of times, from agonizing slow to reasonable, but I don't see a pattern that can reliably replicated.

@aaronjudd aaronjudd merged commit a639d5e into release-1.8.0 Feb 15, 2018

4 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
WIP ready for review
Details
ci/circleci Your tests passed on CircleCI!
Details
security/snyk No new issues
Details

@aaronjudd aaronjudd deleted the refactor-3600-spencern-upgrade-meteor-1.6.1 branch Feb 15, 2018

@spencern spencern referenced this pull request Feb 16, 2018

Merged

Release 1.8.0 #3645

@spencern spencern referenced this pull request Mar 9, 2018

Merged

Release 1.9.0 #3941

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.