Add function names to module factories in DEV

[gaearon]

This adds path-based function names to the generated module factories in development.

__d(function src_some_folder_path_MyComponent_js__module_factory__() {
  // ...
})

They're only emitted when minify is false and dev is true.

Motivation

Profiling module initialization is a common need. The current set of tooling React Native provides for it is arguably abysmal.

The primary tool suggested by React Native documentation is Systrace which literally does not exist anymore. "Profiling with Hermes" suggests using sampling profiler which seems to barely work — I wasn't able to capture any information with it.

If you try to use Hermes Chrome DevTools integration and press "Record" in the Performance tab, it hangs.

I almost gave up at that point, but @brentvatne kindly suggested that there is a workaround — I need to Shift+Cmd+P to get a different "JS Profiler" Chrome DevTools tab to show up, and that one actually works.

Even that approach is a little bit broken but it's the closest to what I want.

Unfortunately, even that tool does not let me see which modules I'm spending time initializing:

As you can see, module factories have blank function names.

This is not the case with web-based tooling. For example, webpack emits module factories as a dictionary like { "./src/some/module.js": function() {}, ... } so the stacks are useful. Since we don't want to change the format entirely, and configuring fn.name dynamically at runtime does not make it show up in the trace, I opted for a minimal possible change — to actually emit the module names in development.

As a result, I can see where I'm spending time during module initialization:

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (6151e7e) 83.09% compared to head (eaf9dd1) 83.10%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1130 +/- ## ======================================= Coverage 83.09% 83.10% ======================================= Files 206 206 Lines 10547 10552 +5 Branches 2617 2619 +2 ======================================= + Hits 8764 8769 +5 Misses 1783 1783 ``` | [Files](https://app.codecov.io/gh/facebook/metro/pull/1130?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=facebook) | Coverage Δ | | |---|---|---| | [packages/metro-transform-worker/src/index.js](https://app.codecov.io/gh/facebook/metro/pull/1130?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=facebook#diff-cGFja2FnZXMvbWV0cm8tdHJhbnNmb3JtLXdvcmtlci9zcmMvaW5kZXguanM=) | `87.19% <100.00%> (+0.15%)` | :arrow_up: | | [...ges/metro/src/ModuleGraph/worker/JsFileWrapping.js](https://app.codecov.io/gh/facebook/metro/pull/1130?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=facebook#diff-cGFja2FnZXMvbWV0cm8vc3JjL01vZHVsZUdyYXBoL3dvcmtlci9Kc0ZpbGVXcmFwcGluZy5qcw==) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[facebook-github-bot]

@robhogan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@robhogan merged this pull request in facebook/metro@2f62858ae187bbb2f49c1e90e4bdca51659d80cb.

[facebook-github-bot]

This pull request has been reverted by 92340b350a5c4b948b989f6c6e444825d5e0aaa6.

[robhogan]

Urgh - this caused native crashes across multiple apps in debug builds post-land, still connecting the dots but there are runtime mem allocation failures coming from Hermes - first few stack frames, FYI:

SIGABRT:libxplat_hermes_API_HermesAPIAndroid.so:./xplat/hermes/external/llvh/include/llvh/Support/MemAlloc.h:llvh::SmallVectorBase::grow_pod(void*, unsigned int, unsigned int)

hermes::irgen::ESTreeIRGen::serializeScope(hermes::ScopeDesc*, bool)
hermes::irgen::ESTreeIRGen::setupLazyScope(hermes::ESTree::FunctionLikeNode*, hermes::Function*, hermes::ESTree::BlockStatementNode*)
hermes::irgen::ESTreeIRGen::genES5Function(hermes::Identifier, hermes::Variable*, hermes::ESTree::FunctionLikeNode*, bool)

[gaearon]

Is there any chance that some (relative) paths are still incredibly long, or that Hermes has a limit on function names that isn't present in other JS engines? I was thinking of adding .slice(0, 50) or something.

[robhogan]

Is there any chance that some (relative) paths are still incredibly long, or that Hermes has a limit on function names that isn't present in other JS engines? I was thinking of adding .slice(0, 50) or something.

String length is a factor, but probably not the determining one. We saw total RAM on Facebook Android (dev builds) go from 1.6GB to 3.8GB with this change, so way more than just the sum of those strings, even though the paths are pretty long.

@tmikov has been working on it and identified a quadratic growth situation where scopes were repeatedly re-serialised during lazy evaluation, but the fix for that didn't seem to be enough on its own.

Worst case, we could enable this behind a serializer config flag in Metro and document the memory use, but the Hermes folks were actively looking into it last I heard so they may want a bit more time.

[tmikov]

There is no limit, Hermes is just not optimized for running from source - this problem doesn't exist when running from bytecode. Running from source with lazy compilation is, in a sense, contrary to Hermes's mission of being an optimizing AOT compiler. We basically bolted lazy compilation on top of the AOT compiler and had to use a lot of hacks, because it simply wasn't designed for that. It is a miracle it has worked as well as it has...

We will support this properly in Static Hermes (at tremendous complexity cost), but for now we are trying to figure out exactly what is consuming the memory in "regular" Hermes. There is something quadratic. (We already fixed one quadratic case, but sadly it made no difference.)