Summary: With libc++ we now have `std::to_string`.
Reviewed By: tmikov
Differential Revision: D31025887
fbshipit-source-id: 53049953457002bf3c3f9bcf659b2797c4d8faf1
Summary:
These libraries are only really used in the PlatformUnicode target, so
there is no need to link them into everything.
Reviewed By: tmikov
Differential Revision: D30657552
fbshipit-source-id: 00e4746251b458d5937e8e5aaccaaaa694bb9f05
Summary:
Add the ability to build libhermes.so with a lean VM in CMake. This
reduces the size of the Hermes binary by ~25%.
Aside from libhermes itself, the distinction is made by having separate
targets, which allows tests to continue running correctly. However, in
order to keep the library name correct, we have to toggle the build
type of libhermes using a build flag.
Reviewed By: kodafb
Differential Revision: D30329792
fbshipit-source-id: c51f5b02d132993a04070a09d19d67c990c26245
Summary:
JSI already exports its headers from the `jsi` target, so there is no
need to explicitly include them. This diff also sets up the same thing
for `hermesapi`.
Reviewed By: kodafb
Differential Revision: D30347507
fbshipit-source-id: 07e8d31b1c680bbe21a34a851278db3ab9c254b0
Summary:
Make the following simplifications:
1. Take the linker flag overrides on libhermes and pass them directly
as arguments to target_link_libraries.
2. Remove the unused `OPTIONAL_JNI_ONLOAD`.
Pull Request resolved: https://github.com/facebook/hermes/pull/568
Reviewed By: kodafb
Differential Revision: D30239469
fbshipit-source-id: ac00259b69a677b73d0cd37491ba3cd39c23276f
Summary:
Make the following simplifications:
1. Remove the `CREATE_SHARED_LIBRARY` flag. It is currently being
passed in only for the static library `hermesapi` and not for
`libhermes` (which seems incorrect). For consistency across MSVC and
non-MSVC, we might as well remove the flag and always define
`HERMES_EXPORT`.
2. Remove custom compile flags from `hermesapi`, since `libhermes`
uses the same sources and currently builds fine without them.
3. Remove explicitly setting the `CXX_STANDARD`, which is now 14
across our code.
Reviewed By: kodafb
Differential Revision: D30242173
fbshipit-source-id: 06fee12b89c74b3085a8899cfe325f78102de785
Summary:
We no longer need to export all symbols in libhermes, since things are
explicitly exported with HERMES_EXPORT. This saves us exporting
HermesRuntimeImpl.
Reviewed By: tmikov
Differential Revision: D30196880
fbshipit-source-id: d1db9c2f7bc91c62e4099ee013682b22fed221c6
Summary:
This PR is https://github.com/facebook/hermes/issues/475 with adjustments
to folder names. It also fixes caches and CI issues.
Pull Request resolved: https://github.com/facebook/hermes/pull/546
Test Plan: CircleCI
Reviewed By: mhorowitz
Differential Revision: D29597379
Pulled By: Huxpro
fbshipit-source-id: f278f9c5503965cb998872993a1ce38cfcdaadcc
Summary:
Custom tags made with `VM_MAKE_TAG` are meant to be used to tag
anonymous mappings, and should be passed in as the file descriptor.
We can't use them for file backed mappings.
Changelog: [Internal]
Reviewed By: kodafb
Differential Revision: D29038256
fbshipit-source-id: 11324a3e798e695709d7d340e84e5fe6f7ffaad3
Summary:
Instead of including JNI source code in our repo and statically
linking in parts of JNI, use the prebuilt shared library available
from Maven.
While working on fixing the `Intl.NumberFormat` crash in the GC, I
got multiple weird exceptions with stack traces that didn't line up
with `hermesLog` statements I had added to the code. I eventually
suspected something was wrong with having multiple copies of JNI, and
switching to the shared library from maven resolved the crashes.
Reviewed By: tmikov
Differential Revision: D28791954
fbshipit-source-id: 992bc04428795cc3154a229211c1a6f24add32d6
Summary:
When an API request to compile JS fails, include in the error message some information about whether the input buffer is writable or read-only, private or shared, etc.
This is meant to help track down issues with buffer contents being corrupted.
Reviewed By: mhorowitz
Differential Revision: D28654598
fbshipit-source-id: b9fdd59a8e7cd775bab533789ac6d65364ce9993
Summary:
`WeakRoot` is more appropriate for `WeakObject` since they are
treated and marked as roots. They don't need the `WeakRefSlot`
indirection since we always know where they are and how to update
them.
This also lets us separate the `WeakRootAcceptor` from the
`WeakRefAcceptor`, which speeds up YG collections (see next diff).
Reviewed By: dulinriley
Differential Revision: D28339576
fbshipit-source-id: 13f48439030500434eaacb7542685575bb5e6ebd
Summary:
Add some new APIs introduced to map a unique ID back to a value.
Also allow getting a unique ID of an arbitrary jsi::Value. Some values don't
have IDs, so return 0 in that case.
If an object can't be found for an ID, return JS `null`.
Reviewed By: neildhar
Differential Revision: D28033006
fbshipit-source-id: e039dd9f3c53e24c5ebb46e5a08d4fa745929b8b
Summary:
A NaN in a JSI value does not necessarily have to be the quiet NaN,
so we need to check and convert it if necessary.
This has likely been the cause of a few crashes, since bad NaN values
could look like object pointers if they are treated as
`HermesValue`s. However, they would have been really hard to diagnose
since they would end up as bad values on the heap that then later
cause a crash somewhere else (like in the GC).
However, since HV32 needs to compress every pointer, the crash ended
up occurring at the compression step instead, which gave us a clear
stack trace pointing to external doubles as the culprit.
Reviewed By: tmikov, avp
Differential Revision: D27960846
fbshipit-source-id: bb9881d0c82f70e68edfa05c0a31b03d67cd2300
Summary:
This diff implement the new JSI API `drainMicrotasks` by draining
the internal job queue and throw JSIException.
Reviewed By: dulinriley
Differential Revision: D27731602
fbshipit-source-id: b2c99e516bd183df37778b25d4202b1e0dd077eb
Summary:
Changelog:
[Internal][Added] - Introduce drainMicrotasks to JSI
This diff proposed a new JSI API `drainMicrotasks` to define
how hosts may integrate with the JSVMs' internal microtask
queue (a.k.a. job queue) and hence their native Promise.
The name `drainMicrotasks` is slightly preferred over `drainJobs`
to favor *host-friendliness* over *engine-friendliness*.
This diff auto implement the new API in JSC and Hermes as stubs
to make sure things compiled.
Please refer to the doc-comments in the `jsi.h` for a detailed
documentation on the semantics and the design of this API.
### Notes on the existing APIs from JSVMs
The presence of such queue and APIs to operate on them are
ubiquitous:
- Hermes: `Runtime::drainJobs`
- V8: `MicrotaskQueue::PerformCheckpoint`
- JSC: `VM::drainMicrotasks`
- QuickJS: `JS_ExecutePendingJob`
The only exception is ChakraCore, which requires hosts to provide
the queue and set up the `JsSetPromiseContinuationCallback`,
but a JSI implementation can provide that queue trivially.
### Extra note on ECMA-262
ECMA-262 changed the spec at Mar 2020 from "asking an ECMAScript
implementation to maintain a Job Queue" to "Ask the embedder to
define its event loop, including the job queue" so technically:
- the internal approach is closer to the old spec.
- the ChakraCore approach is closer to the current spec.
Reviewed By: mhorowitz
Differential Revision: D27727920
fbshipit-source-id: b839b959facbc009f7d14b781e9287c46ea64373
Summary:
Given the build time regression preventing Hermes Modules to ship in debug builds in the near future (more info: https://fb.workplace.com/groups/684200151742597/permalink/1876367169192550), we need to break our dependency on this work for the Jest E2E coverage collection. Because the SegmentID isn't populated in classic builds, the API will return bogus data for us unless we're using Hermes Modules.
To circumvent this, I've created a workaround in this diff that attempts to extract the SegmentID from the SourceURL. This is less than ideal obviously, but without it we cannot ship coverage collection for FB4a, which means we can't do landblocking test runs on React Native product changes. As the usage of Jest E2E is growing, this is causing increasingly large oncall loads for both us (Failing tests blocking OTA updates) and product teams (Oncall having to investigate, bisect and fix failures in the test). For that reason, I think it's worth putting this workaround in place until we have Hermes Modules rolled out successfully.
Reviewed By: makovkastar
Differential Revision: D27293987
fbshipit-source-id: 0090e30dc956ecfd375f496904351f48a6ca1a1b
Summary: The `HermesRuntime::getExecutedFunctions` call only returned information from the first runtime. We're interested in specific runtime(s), so in this diff I've modified the `getExecutedFunctions` method to return a map of `<string -> long[]>`.
Reviewed By: dulinriley
Differential Revision: D26050062
fbshipit-source-id: 255511c836f3021da9a137b12a298e4c9b63abbd
Summary:
Runtime optimisations need to be enabled in order for CompileJS to
actually optimise bytecode. Make this clear in the doc comment since
it's easy to make this mistake.
Also clean up an old comment about a task that has since been closed.
Reviewed By: avp
Differential Revision: D27829745
fbshipit-source-id: b93fdc42ea30c218899a9de9ed0f4e942aa4bcda
Summary:
Although it's not a goal to enforce encapsulation inside the VM,
this diff performs some cleanup to help with
1. Readability: private and public in Runtime are currently interleaved
back and forth, making code unpleasant to follow.
2. Restoring unintentional access: there was also a typo unintentionally
changed many private fields to public, resulting some later code
unintentionally depending on the public access. Note that it's okay
to mark a field public but we don't it happened unintentionally.
Reviewed By: avp
Differential Revision: D27522200
fbshipit-source-id: 63214ef7608da84f1568e546fb359e24018dda3b
Summary:
Add a new action for taking a sampling profile of CPU time
for synth benchmarks.
Use with:
```
buck run synth_target -- -action-at-marker=sample-time [-marker=tti]
```
Any other marker works fine as well, including the "end" marker (meaning the whole
trace).
Reviewed By: neildhar
Differential Revision: D27601474
fbshipit-source-id: d83913bc871909029c22062b7b100a89242522c0
Summary:
The latest versions of clang have a range loop analysis warning. Fix
it in some Hermes code. But there are also uses in LLVH (including in
header files), so turn off the warning to avoid generating noise.
Reviewed By: avp
Differential Revision: D27556102
fbshipit-source-id: 5cdb4cfa1e9597d1b918f136c25c28169d441536
Summary:
Pull Request resolved: https://github.com/facebook/hermes/pull/479
Symbol in Hermes has been stablized for a while and it's enabled
everywhere by defualt. It's now time to clean those dead code.
Reviewed By: tmikov
Differential Revision: D27385604
fbshipit-source-id: 1e48e97771e091870b0d78800aac44d5e3901c29
Summary:
Intl API is standardized under ECMA-402 and it's not associated
with ES6 (ECMA-262 6th).
Reviewed By: mhorowitz
Differential Revision: D27385605
fbshipit-source-id: 5a1d79a5783b8dc0f0220f075355ac895cbbbccb
Summary:
Update all callers of JSArray::create to directly use the handle
returned by it.
Reviewed By: avp
Differential Revision: D27356457
fbshipit-source-id: 06e3fcbb57176693ce56c3fe1dbc84323894cbdb
Summary:
To further test the sampling heap profiler, and allow runners of synth
benchmarks to get some flame graphs of memory allocations,
add a new action to synth benchmarks to take a sampling memory profile.
This can be done with `-action-at-marker=sample` which means to stop sampling
and get the results once the marker is hit.
Reviewed By: neildhar
Differential Revision: D27086260
fbshipit-source-id: 92a384b1400f8b003bfed79a2f767757229dc6b7
Summary:
The sampling heap profiler will need to be enable-able by the
inspector, and the previous snapshot/profiling APIs are all
defined on `jsi::instrumentation`.
Follow their lead and define the new sampling heap profiler APIs on
`jsi::instrumentation` as well, and implement it for Hermes.
Changelog: [Internal]
Reviewed By: neildhar
Differential Revision: D26835152
fbshipit-source-id: 4625be3214297489e04a4ceea8a22622d14299c4
Summary:
The documentation of jsi::Value::strictEquals was confusing it with SameValue (which differs in treatment of signed zeroes and NaN). Fix it by referring to the correct part of the spec.
Changelog:
[Internal][Fixed] - Fix incorrect comment
Reviewed By: avp
Differential Revision: D26430216
fbshipit-source-id: 0ea4208fbdda40c87f8028231ddefb2323b6eb96
Summary:
Comparing floating point values exactly is unreliable, especially
across machines.
Reviewed By: dulinriley
Differential Revision: D26901180
fbshipit-source-id: 76df0eb0b13dff96f4bebe08ec3c9dc649ffd343
Summary:
Pull Request resolved: https://github.com/facebook/hermes/pull/466
For buck, this change makes no difference, since the flag is set to
true by default.
For CMake, this means that we will be able to build the synth tool or
create synth traces without needing to specifically set this build
flag. More importantly, it ensures that the CMake build doesn't break
because it will be continuously built through CI.
There should be no effect on the binary size of hermesapi or
libhermes because the synth trace targets are not linked into them.
Reviewed By: dulinriley
Differential Revision: D26771883
fbshipit-source-id: 1332e39d6cc4ed20b378ab9da41c4be8c9ee90bd
Summary:
We currently cannot build synth trace tests and libraries with GCC.
`SynthTraceParser.cpp` relies on LLVHSupport, which is built without
RTTI. This means that `SynthTraceParser.cpp` itself has to be built
without RTTI. However, it does use exceptions so we have to create a
new special case for it where exceptions are on but RTTI is off.
The synth trace tests are somewhat more complicated. They rely on
LLVH, RTTI, exceptions, and the hermes API simultaneously. Getting
them to build reliably will be much more challenging, and may require
refactoring them to depend on folly JSON instead of the JSON parser
in Hermes, or untangling all of these features somehow. Either way,
for now, I propose removing them from the OSS tests, and only testing
them in buck, like we do for the heap snapshot tests.
Reviewed By: dulinriley
Differential Revision: D26809542
fbshipit-source-id: 35b00f6e4385ef816e381c73dd8ee28646782907
Summary:
Add an option that removes the requirement that the input bytecode was compiled from the same source used to record the trace. There must only be one input bytecode file in this case. If its observable behavior deviates from the trace, the results are undefined.
This option is intended to be used when testing/debugging changes to Hermes that involve changes to source transformations.
#utd-hermes-ignore-windows
Reviewed By: dulinriley
Differential Revision: D26793935
fbshipit-source-id: b1747ec4c1db2bd5d91f472db7d4ecd40d21bb8c
Summary:
Pull Request resolved: https://github.com/facebook/hermes/pull/462
We currently fail to build `hermesAPI` with CMake when synth traces are
enabled because we do not link `hermesAPI` against
`hermesInstrumentation`. Fix this by matching our CMake build more
closely with our Buck build, where we have a separate
`TraceInterpreter` library, that is then linked against by anything
that needs to use it.
We can rely on the ifdefs inside the cpp files to just omit the
source code when tracing is disabled.
Reviewed By: dulinriley
Differential Revision: D26760317
fbshipit-source-id: 9900d4afec4cc41079d0f7cd38b125b2a5d9278e
Summary:
In order to find the last def that occurs at or before the first use,
we can use a `std::set` and `std::set::upper_bound`. This is clearer
and fixes a bug where we currently do not consider defs that occur at
the first use (instead only considering defs before it).
Reviewed By: dulinriley
Differential Revision: D26697839
fbshipit-source-id: 1f1cd5c6a96cf4f3bd799e53e37520db5df182de
Summary:
An assert was incorrectly computing the last def before first use, causing false assert failures.
#utd-hermes-ignore-wasm
Reviewed By: dulinriley
Differential Revision: D26695876
fbshipit-source-id: 1d81b54dc97e46bb48a9b3d5d8a18000d664411f
Summary:
Assert that the actual types and non-pointer values seen when replaying a trace match the expected ones from the trace.
For now, exclude `this` in native calls, due to an issue that was only recently fixed (see D26387774).
Reviewed By: dulinriley
Differential Revision: D26427340
fbshipit-source-id: 5897a622c973da5f8b928249c2eb6e53b13735de
Summary:
When parsing a synth trace in a dev/dbg build, perform basic validation on the format of values, detecting trailing garbage, negative or non-numerical IDs, etc.
Update a few old unit tests that specified strings with their content rather than an ID.
Reviewed By: neildhar, dulinriley
Differential Revision: D26407225
fbshipit-source-id: 7215c6415228535194aaa72ec623f0241d02c149
Summary: The tracing runtime incorrectly assumed host functions could not have a 'this' value, always outputting 'undefined' into the trace. Fix this and add a unit test.
Reviewed By: dulinriley
Differential Revision: D26387774
fbshipit-source-id: 76592da71fa587d26cd5ba5315d0cab761b41ff2
Summary:
`jsi::PropNameID` is a more efficient way of passing a string to various
API checks like `hasProperty` and `setProperty`.
Passing a `const char *` results in extra strings being created on the heap.
Reviewed By: mhorowitz
Differential Revision: D26357795
fbshipit-source-id: ff1b83ed9fe0cb3b2e419b0868ccc877a5540d99
Summary:
If the RuntimeConfig fields "ES6Promise" or "ES6Proxy" exist in the
original trace, replay with those fields set as well.
This avoids accidentally running a trace with Proxy turned on when it was
originally taken with Proxy turned off.
Recording is already implemented in `SynthTrace.cpp`, this change handles replay
In practice this didn't cause any issues, but in general features like Proxy might
have changed the sequence of API actions, and should be recorded and replayed
properly.
Reviewed By: kodafb
Differential Revision: D26349818
fbshipit-source-id: 540733f9ce0d7c4db82c08f7c380d9c231039ab3
Summary: Condition enabling StackRuntime on it not being an fbcode build.
Reviewed By: dulinriley
Differential Revision: D26332858
fbshipit-source-id: 0efa28ed8b3a846affbb69531bf0ad95aff0fe07
Summary:
Add a Hermes-specific API to evaluate JavaScript with an explicit source
map.
Reviewed By: mhorowitz
Differential Revision: D26129974
fbshipit-source-id: 6d9c4d3b2503255fa6718876249dd019f697b19d
Summary:
To get better debug information, and preserve access to the state of
the Hermes runtime, put the Hermes runtime in a stack in its own
thread.
Reviewed By: mhorowitz
Differential Revision: D26233606
fbshipit-source-id: a034dd2788f9e4db2123bea65e7053114bd33740
Summary:
JSI had its compiler options like `-Wno-non-virtual-dtor` exported to targets
that depended on the JSI library.
These flags should be private to the compiler invocation on the `jsi.cpp` translation
unit and don't need to be shared with any libraries pulling in `jsi.h`.
Fixes#448
Changelog:
[Internal] - Don't export C++ warnings from JSI CMake files
Reviewed By: Huxpro
Differential Revision: D26212636
fbshipit-source-id: bd95580f89694189975a49660c439e4e5f7c5afc
Summary:
fix typo in comments
## Changelog
fix typo in comments
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[CATEGORY] [TYPE] - Message
Pull Request resolved: https://github.com/facebook/react-native/pull/30777
Reviewed By: RSNara
Differential Revision: D26108277
Pulled By: PeteTheHeat
fbshipit-source-id: 616c89263c9320bd914b26c1b814eadf316cf6d8
Summary:
Synth trace replay allowed taking heap snapshots, but didn't allow
taking a heap timeline from when the runtime was created.
This allows us to get heap timeline information from any synth benchmark.
To support this, change the CLI to allow two options:
* `-marker=name`: Stop at the given marker and take some action, defaults to "end"
* `-action-at-marker=action`: Decide which action to do, defaults to "stop"
This replaces the existing `-snapshot-at-marker` flag to be more generic.
It also makes it easy to debug issues in the heap timeline on real React Native
apps without having to build them.
Reviewed By: neildhar
Differential Revision: D26003441
fbshipit-source-id: f80000d9d8d3f28f9efa2266c59a9585a8c7deb9
Summary:
Propagate the value of `emit-async-break-check` to the VM runtime using
a new runtime flag. This ensures that the async break check will work in
code generated by eval() or new Function().
Additionally, save the flag in hermes::facebook::HermesRuntimeImpl so it
can be applied by default to all source compiled using the JSI
interface.
Reviewed By: avp
Differential Revision: D26031515
fbshipit-source-id: 41fb96c7997f2c78c4e4d24e36cbc70b4403b8a0
Summary:
The comments for `ExecuteOptions` were really outdated, and were
specified as `\param` even though they aren't function arguments.
A long time ago they used to be function arguments.
Update them to be more helpful, more accurate, and with better whitespace
readability.
Reviewed By: neildhar
Differential Revision: D26007310
fbshipit-source-id: d713fa0e50270862c8f8d1da17195dc99fc7a0e7
Summary:
There are currently a few issues with supporting multiple runtimes in
the sampling profiler. For instance:
1. There is only a single global list of domains that need to be kept
alive for symbolication, which each runtime marks as roots. To
support multiple runtimes, each runtime should have its own
associated list of domains.
2. The sampled stacks across all runtimes are stored in a single
list, which makes it difficult to pick apart the profiling data for
each runtime.
This diff separates the state of the sampling profiler into two
classes:
## `SamplingProfiler::GlobalState`
This singleton class maintains the global state of the sampling
profiler. It maintains signal handler registration, whether the
profiler is enabled/disabled, and the lifetime of the timer thread.
Each runtime must be registered with this global object. The timer
thread will iterate over all registered runtimes and sample stacks
for each of them.
## `SamplingProfiler`
This class acts as a guard to be stored in each runtime, and
maintains all of the profiler data for that runtime.
Creation/destruction of this class will register/deregister the
associated runtime from the global profiler on the current thread.
Captured stacks and the domains we need to keep alive are stored in
this class, since they are only associated with a specific runtime.
With this implementation, the `SamplingProfiler` object needs to be
recreated whenever the runtime is moved onto a different thread. We
may want to revisit that in future, but it is acceptable for now.
Reviewed By: dulinriley
Differential Revision: D25734463
fbshipit-source-id: cc4c7e2b992279805a959a25ab4239604ac1550e
Summary:
Make some sampling profiler methods static, by calling `getInstance`
internally. These static functions will be the "public" API for the
sampling profiler.
Later diffs will separate out the global state, so making these
functions static is a way of hiding the implementation details of the
sampling profiler.
Reviewed By: dulinriley
Differential Revision: D25728898
fbshipit-source-id: f9f2773dd0501de86299240ae020ff6c0af1fef1
Summary:
Fixes https://github.com/facebook/hermes/issues/420
Apple App Store submission, at least the iOS one, requires this metadata to be specified when only targeting a subset of possible archs.
Pull Request resolved: https://github.com/facebook/hermes/pull/425
Test Plan:
(Taken from https://github.com/facebook/hermes/pull/428)
```
~/tmp » curl -L -O https://12725-154201259-gh.circle-artifacts.com/0/tmp/hermes/output/hermes-engine-darwin-v0.7.1.tgz
~/tmp » tar -zvxf hermes-engine-darwin-v0.7.1.tgz
~/tmp » cd package/
~/t/package » node unpack-tarball.js
~/t/package » /usr/libexec/PlistBuddy -c 'Print :MinimumOSVersion' destroot/Library/Frameworks/iphoneos/hermes.framework/Info.plist
10.0
~/t/package » /usr/libexec/PlistBuddy -c 'Print :LSMinimumSystemVersion' destroot/Library/Frameworks/macosx/hermes.framework/Resources/Info.plist
10.13
```
Reviewed By: mhorowitz
Differential Revision: D25839408
Pulled By: Huxpro
fbshipit-source-id: 8067069d66ea3af36366157ffae95445e9e89392
Summary:
GCs currently don't have access to the VMExperimentFlags. Since I
imagine we will need to do several more GC based experiments in the
coming months, this diff wires up the experiment flags through to
GCBase, so that all GCs have access to the flags.
Reviewed By: dulinriley
Differential Revision: D25703809
fbshipit-source-id: e1060db186218fb99e148ee1d69fbc887797a6e2
Summary:
These base classes were only inherited by a single other class and
not re-used, so they don't need to exist.
They used to exist to support API tracing, but that was moved into
a runtime decorator.
Reviewed By: mhorowitz
Differential Revision: D25786511
fbshipit-source-id: 008fd80309b79d3877deed0ac153e5987b048b88
Summary:
The `hermesValues_` and `weakHermesValues_` lists can be a
considerable size if there's a lot of JSI activity going on in an app.
Add a size estimate as a native node to the heap snapshot.
Reviewed By: neildhar
Differential Revision: D25603047
fbshipit-source-id: cb3dc3abc2fd87b328337c2aa9af5a0dc9b4fd79
Summary:
This PR has fixes to Intl API implementation to make it work properly under release configurations. With this PR, we also start building the Intl code into hermes release packages.
Essentially, the PR has the following changes,
1. Add Proguard annotations to keep the JNI referenced Java symbols from being stripped away in minified packages
2. Ensure fbjni is initialized when loading libhermes.so.
3. Add runtime config switch to control the inclusion of Intl object in the global. We keep it OFF by default.
4. Changes in the gradle build definitions to start building C++ and Java Intl code into the hermes release package. We use product flavors to clearly differentiate the Intl enabled packages for now being. We can probably clean things up once the Intl APIs are stabilized.
5. Updated the documentation (Unfortunately still a docx file, at \lib\Platform\Intl\java\com\facebook\hermes\intl\HermesIntlAPIsImplementationNotes.docx) to include the impact on the size of the application package due to this change.
As the runtime switch is currently turned OFF, the real impact of this change is going to be the increased package size. I'm pasting the measured numbers here for quick reference,
Impact on Application Size
The following numbers are measured using a test application which takes dependency on the Hermes library to evaluate a JavaScript snippet. Essentially, enabling Intl APIs adds 57-62K per ABI.
Product APK Size NOINTL INTL DIFF PERC
ARM64 1,672,235 1,729,579 57,344 3.43%
ARM 1,471,539 1,528,883 57,344 3.90%
X86_64 1,844,255 1,901,599 57,344 3.11%
X86 1,950,739 2,012,179 61,440 3.15%
The overhead is contributed by both compiled native C++ and Java bits.
The uncompressed size of the Hermes shared library got bigger as follows,
libhermes.so Size NOINTL INTL DIFF PERC
ARM64 2,473,760 2,551,592 77,832 3.15%
ARM 1,696,672 1,754,016 57,344 3.38%
X86_64 2,633,528 2,711,368 77,840 2.96%
X86 2,859,916 2,945,936 86,020 3.01%
And the Java bits got bigger as well,
Java Size NOINTL INTL DIFF PERC
classes.jar (in hermes.aar) 559 120975 120,416 21541.32%
classes.dex (intltestapp.apk) 160708 234808 74,100 46.11%
Please note that the application dex file contains non-hermes class files too.
And finally, this is the increase in the final npm package,
NPM Package NOINTL INTL DIFF PERC
214447973 219291220 4,843,247 2.26%
I'm not including the other performance metrics (Memory footprint, latency etc.) with this change because,
1. Intl APIs are currently disabled by default.
2. I've measured some metrics and didn't see anything egregious, but I know a couple of potential improvement opportunities, which I want to plug before sharing the numbers, which i'll do as part of enabling Intl by default.
Pull Request resolved: https://github.com/facebook/hermes/pull/426
Test Plan:
I've a test application which was extremely useful in diagnosing and debugging hermes on Android, which I'll include in another PR.
There is no change in the product code which can affect the functionality, and all the tests are passing.
Reviewed By: neildhar
Differential Revision: D25512578
Pulled By: mhorowitz
fbshipit-source-id: 56dfacba2417003f6d62ca6f2e6a5fc15cc16424
Summary:
Move all overloads used for handling roots into `RootAcceptor`.
Create a new `RootAndSlotAcceptor` that inherits from both
`RootAcceptor` and `SlotAcceptor`, and can be used for most GC
acceptors.
Reviewed By: dulinriley
Differential Revision: D25277113
fbshipit-source-id: 1dda2c2934f0bb844fe5a6d06700b1812e3f81ef
Summary:
Using the trace config for a benchmark should be the default, because it
applies defaults based on when the trace was taken, rather than whatever
Hermes's defaults are now.
You can still test other parameters by explicitly passing `-use-trace-config=false`.
Reviewed By: neildhar
Differential Revision: D24697016
fbshipit-source-id: 9493d78a0c99eccdae0930a51e7123e22eb55454
Summary:
Our current code coverage profiler operates as a singleton and
multiple threads may simultaneously access its data structures. This
diff makes the following changes:
1. Create a separate CodeCoverageProfiler instance for each runtime,
which tracks coverage information for only that runtime.
2. Protect access to data structures with mutexes to allow for safe
concurrent access
3. Add static methods to manage global state, allowing us to keep the
existing external interface and avoid having to refactor the Java side
This change does not aim to make the code coverage profiler actually
work when multiple runtimes are present (there seem to be some limits
in symbolication), but instead to just make it safe to do so.
Reviewed By: dulinriley
Differential Revision: D24357255
fbshipit-source-id: 5c21839693292442d40c2e967efb117e0359b870
Summary:
The current implementation of createBCProviderFromSrc is written with
the implicit assumption that the resultant bytecode will only ever be
used for direct execution, and never for serialisation. Because of
this, it contains a step where it serialises jump tables into the
opcode list so that they can be executed.
However, there are cases where we use the output of this to generate
bytecode bundles (as in compileJS). Before this diff, this had two
issues:
1. Since the jump tables are encoded inline with the opcodes, we
would end up serialising the jump tables into the bytecode twice.
Once with the opcodes, and once again when we perform the jump table
serialisation step.
2. The switch case is going to use the jump table that was encoded by
`createBCProviderFromSrc`. However, that jump table is not guaranteed
to have the 4-byte alignment we expect in memory. This is because the
padding for that jump table was computed only with respect to a
single function and not to the entire program's bytecode. As a
result, the serialised bytecode will not have the jump table at a
4-byte aligned address, and the runtime will crash when trying to
read from it.
This diff fixes the issue by allowing the caller to pass in a
`format` option to `createBCProviderFromSrc`. The format option is
used to determine whether the bytecode is intended for immediate
execution and therefore whether the jump tables should be inlined.
The fix isn't entirely ideal since it essentially involves special
casing the execution case. Instead, I've tried to address the issue
described without disturbing other callsites. A better fix would
probably involve standardising how the BytecodeModule is expected to
store data, I'm working on figuring out the right way to do that.
Reviewed By: tmikov
Differential Revision: D24075278
fbshipit-source-id: 855504abf10a95c45a13198c898ad6379ffb4021
Summary:
`getGCConfig` expected a the JSONObject for `runtimeConfig`, but we were
accidentally passing the root. This error was disguised in the test buy
including the GCConfig into the RuntimeConfig builder, but that doesn't
properly expose the builder interface for GCConfig to allow updates.
Remove the call to `getGCConfig` from within `RuntimeConfig`. It's now
the caller's responsibility to merge these two how they'd like.
Reviewed By: neildhar
Differential Revision: D24697015
fbshipit-source-id: 974b66bb9186055ea959bcf2a0bac8567ce91bc8
Summary:
Changelog:
[General][Fixed] - Fix handling of very deeply nested data across the bridge
fixesfacebook/hermes#339
Reviewed By: sammy-SC
Differential Revision: D24182938
fbshipit-source-id: b674283a112b98cc63f20e436c538e3789ddf6dd
Summary:
- Add the CLI flag `-Xes6-promise` and RuntimeConfig flag `ES6Promise`
- make `HermesInternal.hasPromise()` return true when the flag is set
- add test for promise with `-Xes6-promise` set
Reviewed By: avp
Differential Revision: D20106660
fbshipit-source-id: d3f3535ca6f832d3f56814581a79041bb8d4900d
Summary:
When taking a heap timeline, Hermes wasn't showing any data until the timeline
was written to disk and then reloaded.
Turns out we were missing support for two events:
* `HeapProfiler.lastSeenObjectId`: This event reports the most recently
allocated object ID. Used to know when objects were allocated in the
timeline.
* `HeapProfiler.heapStatsUpdate`: Report how many objects and bytes
exist for a "time fragment", represented by a fragment index. Later updates
to the same index can decrease the amount of live memory, which show up
as grey spikes instead of blue spikes
Previously, we only supported these by writing out to a file, and they didn't work
with a "live" profiling view. To fix this, I changed the periodic sampling thread
to instead be a periodic flush of a sample every few allocations. The performance
impact is tucked away only when profiling is turned on, and it's very non-invasive to
the rest of the GC. The flush calls a callback with the relevant information if the
inspector is on, and the inspector sends a message back to the browser.
Changelog: [Internal] Fix for Hermes heap timeline profiling
Reviewed By: neildhar
Differential Revision: D23993363
fbshipit-source-id: 8e0b571130cbb7e839dfb009b04f584f5179085d
Summary:
Call the flag `enableGenerator`, thread it through the API,
and leave it defaulted to true to avoid any unpredicted changes.
Reviewed By: mhorowitz
Differential Revision: D23946616
fbshipit-source-id: 89f33482ca0888deae6a9e0cdacb42cff47d6a91
Summary:
Bitcode is turned on by default in React Native and so, setting it here as well.
Changelog: [iOS] [Changed] - Upgraded JSI with a new HERMES_ENABLE_BITCODE flag
Pull Request resolved: https://github.com/facebook/hermes/pull/365
Reviewed By: tmikov
Differential Revision: D23823228
Pulled By: Huxpro
fbshipit-source-id: d43638818a733f6a87b2f4a1ecadad8ea9c7a419
Summary:
Continuing the adding of a "cause" field for logging to GCs.
This allows embedders of Hermes (such as React Native) to specify
the cause of a call to `collectGarbage`.
Notably, this allows Hermes to know when a GC is triggered by a memory warning.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D23742099
fbshipit-source-id: 99453e632328c00045b92a72f789d41c898dc518
Summary:
When logging when a GC occurs, it's good to know if the GC was forced,
or was part of the natural allocation paths. This allows us to see if
there are too many forced GCs in apps.
Reviewed By: neildhar
Differential Revision: D23737916
fbshipit-source-id: fd8d6068bed14197b752c3a012f72c1309bc8311
Summary:
Address a number of compiler warnings that appear when building with
GCC.
Reviewed By: dulinriley
Differential Revision: D23754059
fbshipit-source-id: e6a7d0e178a5f13852348a0992669d7e39c8dce2
Summary:
This diff allows specifying the preemptive compilation threshold via CompileFlags.
Users get a toggle in CLI and RuntimeConfig that allows choosing between 1. fully eager, 2. fully lazy, 3. use default thresholds ("smart"). The default is #3.
However, since we use `-O` by default, it overrides any lazy compilation when
using the `hermes` binary.
ChangeLog: [Internal] Inspector updated to use new RuntimeConfig laziness control
Reviewed By: tmikov
Differential Revision: D23356463
fbshipit-source-id: 508b7b2e6a218346c69acfec97e7891e388f0e9b
Summary:
In most cases, we want to always be sure to return upon throwing an error.
In the rare cases we don't want to use the result, add a `(void)`.
Reviewed By: tmikov
Differential Revision: D23629000
fbshipit-source-id: 0951e6c97cb05dd0a152c0f0e6c94046687a139a
Summary:
This PR allows building and running Hermes on iOS devices. It extends the prior work done by alloy that enabled Hermes to run on MacOS.
In a nutshell, the Hermes podspec was extended to include a dedicated framework for iOS devices. That framework itself is a universal framework that includes binaries for different iOS architectures and an iPhone simulator.
Every target is built into `build_<name>`, e.g. `build_iphoneos` and `build_iphonesimulator`. This convention (and helpers that I have created) allows for quick and easy extension to support iPad as well (and probably should happen as a part of this or next PR).
Here's dump of `destroot` folder to give you an idea how frameworks are located:
```
destroot/Library/Frameworks
├── iphoneos
│ ├── hermes.framework
│ └── hermes.framework.dSYM
├── iphonesimulator
│ ├── hermes.framework
│ └── hermes.framework.dSYM
└── macosx
├── hermes.framework
└── hermes.framework.dSYM
```
Things work locally and the test mobile application prints "Hello World" just fine.
I have started an appropriate discussion regarding them around the lines where the todos are located.
As a follow-up,
## Follow-up
- [ ] a PR to React Native to allow using Hermes as a default engine
- [ ] https://github.com/facebook/hermes/pull/332#issuecomment-678109796
## Todo
- [x] https://github.com/facebook/hermes/pull/332#issuecomment-678111329
Pull Request resolved: https://github.com/facebook/hermes/pull/332
Test Plan:
1. Clone the project
2. `cd test/ApplePlatformsIntegrationTestApp/` && `pod install`
3. Open `ApplePlatformsIntegrationTestApp.xcworkspace`
4. Choose `ApplePlatformsIntegrationTestMobileApp` target
5. Run on a device or a simulator
Reviewed By: willholen
Differential Revision: D23320554
Pulled By: mhorowitz
fbshipit-source-id: 806c98257904df426a76b441954061a261f97e86
Summary:
The change in the hermes repository fixes the security vulnerability
CVE-2020-1911. This vulnerability only affects applications which
allow evaluation of uncontrolled, untrusted JavaScript code not
shipped with the app, so React Native apps will generally not be affected.
This revision includes a test for the bug. The test is generic JSI
code, so it is included in the hermes and react-native repositories.
Changelog: [Internal]
Reviewed By: tmikov
Differential Revision: D23322992
fbshipit-source-id: 4e88c974afe1ad33a263f9cac03e9dc98d33649a
Summary:
On windows it is required to explicitly specify what symbols need to be exported from a DLL to make them accessible (using `__declspec(dllexport)`). I have added and expanded on existing macros to do this and added exports that were previously missing.
Changelog:
[Internal][Changed] - Allow Hermes to be compiled to a single DLL on windows
Reviewed By: mhorowitz
Differential Revision: D23084343
fbshipit-source-id: 832cb17b9e637e4c04dad479aae6c1fc190f968e
Summary:
We've observed an issue where exception messages get lost across
the C++ to Java boundary in OSS builds. This is a temporary workaround to log
the exception message so it can be recovered from adb log if nothing else.
Reviewed By: mhorowitz
Differential Revision: D22743630
fbshipit-source-id: 22c7cd3ebb4cbe82634718c8fd2be447c388ddb8
Summary:
Add `HERMES_BUILD_APPLE_DSYM` option to build a dSYM bundle for the libhermes target on Apple platforms.
This will work with any of the build types and is off by default.
Installing the tools with the install/strip target will ensure all tools and the runtime lib are stripped of debug symbols, but leaving the dSYM bundle in tact.
Pull Request resolved: https://github.com/facebook/hermes/pull/296
Test Plan:
### Build
```bash
./src/utils/build/configure.py --distribute --cmake-flags='-DHERMES_BUILD_APPLE_DSYM:BOOLEAN=true -DCMAKE_INSTALL_PREFIX:PATH=../destroot_release' build_release
cmake --build ./build_release
```
### Install without stripping
```bash
cmake --build ./build_release --target install
nm -a destroot_release/bin/hermesc | wc -l
27943
```
### Install with stripping
```bash
cmake --build ./build_release --target install/strip
nm -a destroot_release/bin/hermesc | wc -l
250
```
…and dSYM DWARF metadata is maintained:
```bash
dwarfdump --statistics destroot_release/Library/Frameworks/hermes.framework.dSYM
{"version":3,"file":"destroot_release/Library/Frameworks/hermes.framework.dSYM/Contents/Resources/DWARF/hermes","format":"Mach-O 64-bit x86-64","source functions":30305,"source functions with location":30302,"inlined functions":172725,"inlined funcs with abstract origins":172725,"unique source variables":79276,"source variables":353690,"variables with location":232195,"call site entries":186409,"scope bytes total":19161949,"scope bytes covered":10500176,"total function size":1763513,"total inlined function size":998375,"total formal params":300264,"formal params with source location":166067,"formal params with type":300264,"formal params with binary location":200407,"total vars":38809,"vars with source location":38385,"vars with type":38809,"vars with binary location":22161}
```
Reviewed By: tmikov
Differential Revision: D22576263
Pulled By: willholen
fbshipit-source-id: 2bda49e638d145ba5d029e77069d6adcc0b1dd8c
Summary:
A recent commit broke debugging on MacOS. Revert the changes causing
the problem. A more comprehensive fix is likely to come.
Reviewed By: kodafb
Differential Revision: D22551383
fbshipit-source-id: 0d9c1e69db5a435abe426d4edf0d24b831b115c2
Summary:
Change the LLVH include path from `llvm/` to `llvh/`, rename the namespace to
`llvh`. This should eliminate any conflicts with the "real" LLVM.
(Note: this ignores all push blocking failures!)
Reviewed By: dulinriley
Differential Revision: D22202846
fbshipit-source-id: bbcabd8439e03e1457939ef5a8dad19f1d1a2f5c
Summary:
This adds the necessary scaffolding to build and test upcoming Intl
support for Hermes.
Reviewed By: tmikov
Differential Revision: D22177030
fbshipit-source-id: 2cc636a9ee37a834d25bce82a70e9d6f05a54094
Summary:
Numeric literal separators are a stage 3 proposal:
https://tc39.es/proposal-numeric-separator/
Numeric literals may now include `_` between any two digits.
The `_` is prohibited at the start or end of a numeric literal,
or adjacent to the `.`, `e`, etc.
We avoid incurring too much extra cost here by setting a `seenSeparator`
flag on the first pass through the number before we calculate its value.
In the vast majority of cases, there is no separator and we can use our
usual methods. In particular, the fast pass for <9-digit decimal integers
only incurs one extra check in the branch.
Anything involving the separator is then delegated to `strtod` or to
`parseIntWithRadix`. The former involves copying the characters that
are not `_` to a buffer to pass to the third-party function, while
the latter has been augmented with a flag to enable ignoring `_`
(other users such as `Number()` must still error on seeing `_`).
Reviewed By: tmikov
Differential Revision: D21796446
fbshipit-source-id: a35c0c493224033647235254811eed5688143771
Summary:
This diff made clear that buffer prefix used in error reporting
message should use unsigned bytes and add a test, potentially
fixing signed/unsigned related issues during formating.
Reviewed By: kodafb
Differential Revision: D22009839
fbshipit-source-id: b176efd95e4b03a56a53db6d9fe881d7d19b8231
Summary: Embed a RuntimeConfig instance in ExecuteOptions. Implement a seperate merge function to merge additional options from ExecuteOptions to configFromFile and save the merged RuntimeConfig in ExecuteOptions. The unittest is also updated accordingly.
Reviewed By: dulinriley
Differential Revision: D22022773
fbshipit-source-id: fdbc1d94cbc81fcd738ed5e74d86a883fb3723ce
Summary:
I was going to add support for `createWeakObject` and `lockWeakObject` to synth traces,
but by trying to implement them I realized that they were unnecessary to trace.
I put the reason why in the code comments so that others don't need to think about it
either. I also removed the TODOs since there's no plan to implement these anymore.
Reviewed By: mhorowitz
Differential Revision: D21986513
fbshipit-source-id: f02193df9950c029f2693ff9ca94d92be879aa54
Summary:
This diff add a static method to `HermesRuntime` for the
purpose of querying the supported bytecode version before
initializing the VM.
Reviewed By: mhorowitz
Differential Revision: D21984358
fbshipit-source-id: 200b5abefaa33d4b58946ab713512b98b1a279cf
Summary:
In a concurrent GC like Hades, there is a problem with a
weak reference: reading from it creates a strong pointer to something
that is otherwise unreachable.
Depending on where that value is stored, and how marking is implemented,
there's no guarantee that the now-reachable value will be kept alive.
When a weak reference is read, be conservative and assume that it might get
written somewhere where it will stay alive. So mark it as live there. In the worst case,
this could result in a weak reference staying around indefinitely if it happens to
get read during every collection.
This diff adds a read barrier to all `WeakRef` instances.
This read barrier currently does nothing on GenGC, and will do something
in Hades in a subsequent diff that adds concurrent marking.
It also introduces the `WeakRoot<T>` type, which does read barriers for weak roots.
A second problem with concurrent marking of weak references is that they
live in off-heap data structures like `llvm::DenseMap` that can move or resize
while marking is ongoing. The mutator will share a `WeakRefMutex` with
the GC to make sure that accesses to these data structures are protected.
To enforce this, the read barrier APIs all check the mutex.
This mutex can go away if those external structures are moved
to become on-heap. The mutex is taken by all APIs to ensure no accidents happen
with WeakRef-containing code.
Through the writing of this diff, I found that `WeakRefHolder` and weak ref support
in `HandleRootOwner` were unnecessary, so I deleted them to save some complexity
of supporting a use case that didn't need to exist.
Reviewed By: davedets
Differential Revision: D20526993
fbshipit-source-id: 9f63e7e6872ea7b67a28af1b009d967eae83380c
Summary:
JSError creation can lead to further errors. Make sure these cases
are handled and don't cause weird crashes or other issues.
This solution has a few parts:
* include a ScopedNativeDepthTracker in checkStatus
* If an exception object message or stack property is already a String, don't
call JS String ctor on it
* Verify that a jsi::Value is a String before calling getString on it.
* Add more tests for JSError construction
Changelog: [Internal]
Reviewed By: dulinriley
Differential Revision: D21851645
fbshipit-source-id: 2d10da0e741ad4ede93cd806320f68ad512e5138
Summary: When compilation fails, add the first 16 bytes of the buffer to the error message. This can help diagnose the issue in case we're trying to compile something that's not actually JS source.
Reviewed By: neildhar
Differential Revision: D21852322
fbshipit-source-id: 00b22a56b921ec5721b0121fede599b0d4b96e8d
Summary:
Make most of the JSObject API functions return `PseudoHandle` instead of
`HermesValue`. This reduces the risk of using the result value across
allocations.
Reviewed By: tmikov
Differential Revision: D19746864
fbshipit-source-id: 8acf3f083adb567113529388b2f05a8333e83f01
Summary:
A key difference in WeakRefs with the Hades GC is that they are mutable,
in the sense that reading the value of a WeakRef while a GC is active might
clear the WeakRef.
For this reason, attempting to lock the WeakObject might mutate the backing
reference.
I preferred to communicate this change in behavior via the API rather than
`const_cast` it away inside the implementation.
Changelog: [Internal] Change jsi::WeakObject to be mutable in lockWeakObject
Reviewed By: mhorowitz
Differential Revision: D21485758
fbshipit-source-id: 3618928be8f8791aed56cb20673896ff5b786ded
Summary:
There were two duplicate error messages in `sanityCheck`: "Buffer too small".
In the interest of having unique error messages for unique errors, which makes
triaging the cause easier, make them unique.
Also, since they're saying the size is too small, might as well include the actual
and expected sizes.
Reviewed By: JoshuaGross
Differential Revision: D21553722
fbshipit-source-id: 6bf183b81b6492b4079f3d2242d4192ce41d67f7
Summary:
We consume Hermes through multiple .so's, which means we have multiple (weak) typeinfo definitions of facebook::jsi::JSError. Previously we were using gnustl, which would strcmp typeinfo to decide whether a certain exception handler applies, which meant this didn't cause any major issues. However since this is deprecated, we recently switched to libc++, which does not have this by behaviour (or it does, but behind a flag I'm not sure how to enable). This causes any JS exceptions to fall through from our exception handlers and fatal the app.
This problem is actually documented in the common Android NDK problems page: https://android.googlesource.com/platform/ndk/+/master/docs/user/common_problems.md#rtti_exceptions-not-working-across-library-boundaries
The suggested solution is to ensure that any exception types have a key function defined (a non-pure, out-of-line virtual function). The simplest one to add is a virtual destructor. This makes the object file that holds the implementation of the destructor export a non-weak typeinfo definition which will at load time override the other weak versions.
I'm not sure why we're the first to hit this. RN's JSIExecutor doesn't explicitly reference JSError which probably helps (https://github.com/facebook/react-native/blob/master/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp#L256-L258) and they also don't use unguarded callbacks like we do.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D21426524
fbshipit-source-id: 474284ada1ca2810045dc4402c420879447f9308
Summary:
hermes/hermes.h should not depend on VM internals. Remove
GCExecTrace.h, and replace it with a forward declaration.
To prevent this from happening in the future, refactor the
hermes_api_library macro to remove include:include from exported_deps.
Reviewed By: dulinriley
Differential Revision: D21308540
fbshipit-source-id: 4685c247518162e9108f259d808a8b2c4f0b2044
Summary:
Expose the ability to dump sampling profiler output to a given
stream. This makes it possible to directly extract sampling
information via the API without needing to write it to a file
Reviewed By: haozhun
Differential Revision: D20954418
fbshipit-source-id: 02890671894500674b7da07f82792c59f86372e5
Summary: This is the squash of already approved diffs D19897285 and D21186651; they need to be squashed to make sure there isn't a point where MobileLab tests didn't work.
Reviewed By: dulinriley
Differential Revision: D21194012
fbshipit-source-id: 980ebb3381da5a8d0442dd0cb950231d71c1ad7d
Summary: On D19897285, Riley made a suggestion that was in fact a significant simplification: assigning ObjectID unique IDs to PropNameIDs, using the same mechanism we use for objects. This diff creates the infrastructure for this.
Reviewed By: dulinriley
Differential Revision: D21009213
fbshipit-source-id: 947f3716718799421db73d4a9ca0139c3d395674
Summary: During the course of working on SynthTrace, I noticed a couple of small simplifications that I couldn't resist. This diff separates them out from a larger diff.
Reviewed By: dulinriley
Differential Revision: D20828785
fbshipit-source-id: 95fe4acc8bfe1f1dab9e0e2893866d6057eea946
Summary: I knew theoretically that the way we recorded strings could lead to more string allocations during replay than occurred during original execution. This diff adds a test that demonstrates this problem. Adds a new version of TraceInterpreter::execFromMemoryBuffer that takes an existing runtime, to make the test easier to write.
Reviewed By: dulinriley
Differential Revision: D20608474
fbshipit-source-id: 1ac544e45d7364d51d0f12244ec85e0f6b9f21c0
Summary:
To determine whether we have precisely reproduced the sequence of GCs (and allocations) between the original execution and a trace, we need to keep a record of those GCs (and allocations) in the trace. This diff does that. Note that fine-grained allocations are only tracked prior to the first YG GC for now -- these would make the trace very large.
Currently, this is only used when we compile with a new hermes.api_trace_debug config parameter. In the future, we may record the GC sequence always, and extend it to have the set of Object IDs reachable from external roots, so that we can only use the external roots observed during the original execution.
Reviewed By: dulinriley
Differential Revision: D20448892
fbshipit-source-id: 3ff945a07a281d760aea1afa0900d1f692264719
Summary: A tiny change: when we create the function for adding marker records, use the TracingRuntime to create the PropNameID for the global name of this function.
Reviewed By: dulinriley
Differential Revision: D20821067
fbshipit-source-id: 0606e58a6a56394eb5c0b783b30d27538a0f04e6
Summary: There are several preprocessor flags, and other build attributes, that can affect the behavior of Hermes in the original execution. This diff starts to record the values of those in the trace, so we can know that how we have to build Hermes used in replay.
Reviewed By: dulinriley
Differential Revision: D20818921
fbshipit-source-id: d513207e9953144af1a43e76b32a37f120b4adc8
Summary:
The BeginExecJSRRecord type in synth trace records the hash of the JS source, but it doesn't record whether the original execution was from source, or from the bytecode compiled from that source. This diff adds a boolean property to the record to indicate that.
We will probably use this in the script that collects the source or bytecode from the build artifacts, so it can find the .hbc or .js files, appropriately.
Reviewed By: dulinriley
Differential Revision: D20818603
fbshipit-source-id: 5d05cdbd5786061c8d6b673c1619880c9ce34d39
Summary: Haozhun's recent diffs changed the behavior of TracingHermesRuntime, so that its destructor no longer flushes the trace -- flushing must be done explicitly. When we run synth with -trace, this wasn't being done. This diff fixes that.
Reviewed By: haozhun
Differential Revision: D21193854
fbshipit-source-id: 73b859d6ba4cd334d61b0b3b06c68c9d60e40c8d
Summary:
This PR conceptually factors out the "holds a native pointer" aspect of
HostObject, allowing it to be used independently from the "interposes properties."
The new intermediate class is called DecoratedObject.
This fills an integration gap. Say you wish to attach a native thing to a JS object,
and manipulate the native bits from prototype functions. This is not possible
with HostObject, because the prototype is not searched. DecoratedObject is
the missing intermediate layer: it manages the native bits (memory management,
etc) without affecting the object model.
This PR squashes the following commits:
## Switch HostObject to storing unique_ptr instead of shared_ptr
HostObject currently stores a shared_ptr, but in practice the refcount is
always 1; jsi does not share objects. Switch to storing unique_ptr instead.
This both reduces allocation size and prepares for introducing
DecoratedObject.
## Introduce DecoratedObject
Hermes currently has no easy way to associate a finalizable pointer with an
JSObject. This commit introduces a new JSObject subclass, called
DecoratedObject, to fill this gap.
A DecoratedObject holds a unique_ptr to a Decoration, which is intended to
be subclassed. There are ways of getting and setting the Decoration, and
also for the Decoration to report its memory usage.
The Decoration is purely for "integration" uses. JSObject does not care about
the decoration.
The next commit proposes rebuilding HostObject atop DecoratedObject, which
reduces some duplication.
## HostObject to subclass DecoratedObject
This "rebuilds" HostObject on top of DecoratedObject. HostObjectProxy now
subclasses DecoratedObject::Decoration. The advantage is to reduce code
duplication: HostObject can take advantage of the memory management
of DecoratedObject.
The third commit rebuilds HostObject on top of DecoratedObject; now HostObject only does the property-interposing parts.
Pull Request resolved: https://github.com/facebook/hermes/pull/225
Reviewed By: tmikov
Differential Revision: D21211233
Pulled By: mhorowitz
fbshipit-source-id: 2e3e1100f00ed765df85be19231744445c328f1c
Summary:
Previously, a derived class, WithTuple, was used. This ran
into bugs in MSVC (see https://github.com/microsoft/STL/issues/121).
Instead, use specialization to get the same result using std::tuple
directly. This avoids the bug, and is a cleaner API.
Changelog: [Internal]
Reviewed By: dulinriley
Differential Revision: D21233677
fbshipit-source-id: 1d75991847164e525b4ba70f65a90627e5f8cd56
Summary:
I'm trying to use JSI for a React Native custom module. I saw these existing examples where the JSI API is used in the context of a CMakeLists.txt:
https://github.com/terrysahaidak/host-object-test/blob/master/libs/android-jsi/test-jsi/src/main/cpp/CMakeLists.txthttps://github.com/ericlewis/react-native-hostobject-demo/pull/4/files#diff-834320be1b4e4016bac27c05dcd17fb9
In both cases, they manually grab the include directories and jsi.cpp from node_modules/react-native, but I also saw that node_modules/react-native/ReactCommon/jsi/jsi already has a CMakeLists.txt that appears to be intended to provide a jsi static lib, so I tried to pull this into my own CMakeLists.txt like this:
```
add_subdirectory(${RN_DIR}/ReactCommon/jsi/jsi ${CMAKE_CURRENT_BINARY_DIR}/jsi)
...
target_link_libraries(MyLib jsi)
```
Unfortunately when doing this, the consuming project still doesn't see the correct include directories. The change I'm proposing here is to use `target_include_directories` and declare that `..` is a public (to consumers) include directory for the library named `jsi`. With this change, you can do what I showed above to consume the jsi lib by just pulling in the CMakeLists.txt file into your own CMakeLists.txt file.
Changelog: [General][Fixed] Fix jsi cmake include dirs
Pull Request resolved: https://github.com/facebook/hermes/pull/207
Reviewed By: willholen
Differential Revision: D21074270
Pulled By: tmikov
fbshipit-source-id: 7d9ec3255f57a16c0b2be489dffa4540727738a1
Summary: Utilize the new factory so that synth trace will first be written to a temporary path, and then renamed at completion.
Reviewed By: davedets
Differential Revision: D20749782
fbshipit-source-id: 790fbab2f20aa6aa23501d1c980f52a5a528b670
Summary:
* With the new API, synth trace will first be written to a temporary path, and then renamed at completion. This is necessary to avoid uploading (and deletion) in-progress synth trace files.
* It also adds a finish callback to indicate completion of a synth trace file.
Reviewed By: davedets, dulinriley
Differential Revision: D20828843
fbshipit-source-id: 0db9e42fa0ba71a710ae36b0eaa2a59c76ff065f
Summary:
Code that consumes these new fields are in an upcoming diff. The new fields are added, so that:
* synth trace will first be written to a temporary path, and then renamed at completion. This is necessary to avoid uploading (and deletion) in-progress synth trace files.
* The rewrite adds a finish callback to indicate completion of a synth trace file.
Reviewed By: dulinriley
Differential Revision: D20828842
fbshipit-source-id: c2bdf65fc7623c627d551800750b77ec55723021
Summary: Catch std::runtime_error thrown by CompilationManager::compile (used for imports) or jsi::JSINativeException thrown by HermesRuntimeImpl::prepareJavaScript (used to evaluate additional JS) and rethrow a HaaSExecException with type COMPILE.
Reviewed By: davedets
Differential Revision: D20754905
fbshipit-source-id: 76a94b0b53032331163ec49186a7944ed3bdb13b
Summary: Add appropriate files to the CMake project when HERMESVM_API_TRACE is set.
Reviewed By: dulinriley
Differential Revision: D20751415
fbshipit-source-id: 919342ac71dc01c5409707a3e722f1a004cead42
Summary:
If name is passed in as part of RuntimeConfig, that is included
in the description, too.
Changelog: [Internal]
Reviewed By: dulinriley
Differential Revision: D20716320
fbshipit-source-id: f2fba6df32f496090dee787d8b7f55a6a4dd8ed8
Summary:
This was causing an exception cascade leading to production
errors. Added a test which repros the problem and passes with the
fix.
Changelog: [Internal]
Reviewed By: tmikov
Differential Revision: D20408858
fbshipit-source-id: 3fa9b8669bf3bf7617bfc05ef8f23d52bc969b4e
Summary:
Experiments show that the config used during replay is *not* entirely the config recorded in the trace. The "synth" program allows a large set of runtime flags, and those of these that are acted upon overwrite the trace's config with the flag's default values. This is not good for replay determinism. (Nor is it good, really, for benchmarking -- we really want to be running with similar settings, I think.)
This diff only overrides the config values from the trace if the corresponding command line flag is explicitly specified. After searching a bit, I figured out that llvm:🆑:opt::getNumOccurrences() allows you determine this. The fields in TraceInterpreter::ExecuteOptions that correspond to config parameters becomes LLVM::Optionals, with default value None. We only override the config parameter if the ExecuteOptions field has a value.
Reviewed By: dulinriley
Differential Revision: D19910876
fbshipit-source-id: f95cede57bf85b5fd3e65f39c32655a34d854934
Summary:
The trace interpreter, when in -trace mode (where we output a trace of the trace), was emitting marker records unnecessarily. We noticed this in trying to get to "exact trace reproducibility": every "tti" or "ttrc" that appeared in the original trace appeared twice in the trace of trace.
This turns out to be because the TraceInterpreter re-emits the markers when it encounters input markers. This may have been necessary at some point, but it isn't now: the TracingRuntime binds __nativeRecordTraceMarker to a host function that emits the marker, and the JS calls this method during replay.
But this turned out to be wrong in a variety of ways. In particular, the object ID for the add marker host function comes "out of thin air." The new plan is:
* In the original execution, we use the TracingRuntime to create the host function, and bind it to the property name. So these show up as records in the original trace. Further, since it occurs with CallToNative and ReturnFromNative records, so the marker record is processed during replay at the right point in the trace.
* We pass a flag down to TracingRuntime's makeHermesTracingRuntime indicating whether the runtime is being created "for replay." In that case, we do *not* add the marker function. The trace interpreter will simulate the add, and we leave the code to echo the recorded markers as it was.
Reviewed By: dulinriley
Differential Revision: D19906411
fbshipit-source-id: 5e74dbab28f5be7355b11e1d3aa4b022f459b35c
Summary: We weren't writing the object ID out in the JSON form of HasProperty records. This caused a synth trace with HasProperty records to be unparseable.
Reviewed By: dulinriley
Differential Revision: D20285966
fbshipit-source-id: 77a062286613fcb729da1720634b5c4e233d1d8a
Summary: It's useful to be able to determine whether a tracing runtime was created by searching logcat. (In case, for example, you set the MobileConfig parameter, but are confused about whether you need to restart or not...)
Reviewed By: dulinriley
Differential Revision: D20285841
fbshipit-source-id: e6603417d12fcd7c0b449a8676050810429222ca
Summary: This is the second diff related to getting Tracing to work with Fabric. Experimentation shows that the jsi::Runtime& passed to operations of TracingHostObject is not always the TracingRuntime; sometimes it's the HermesRuntimeImpl stored in JsiProxy. Therefore, we follow the pattern of DecoratedHostObject, and store a reference to the TracingRuntime in the TracingHostObject, and use that for the tracing-related operations, and pass it down to any calls.
Reviewed By: dulinriley
Differential Revision: D20181848
fbshipit-source-id: 555e73437f3168644be87a434300066d62dc7efb
Summary:
I was unsure how, or if, DecoratedHostObjects worked. Marc had created the linked task to make sure that things worked right with multiple levels of decorated host objects. I decided to do that, thinking it would show a bug; instead it showed me that the pattern did work, and let me figure out how.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D20263797
fbshipit-source-id: 8bbe71cba1b6ef5b2673566759112320dd1894b8
Summary:
Before this diff, TracingRuntime invoked RuntimeDecorator::createObject, which added an extra layer of delegation. getHostObject did not undo this.
With this diff, TracingRuntime::createObject(shared_ptr<HostObject>) no longer invokes RuntimeDecorator::createObject. Instead, it directly invokes the createObject of the underlying "plain" runtime. Thus, it avoids adding the extra level of delegation, and getHostObject now correctly yields the underlying host object.
Reviewed By: mhorowitz
Differential Revision: D20181834
fbshipit-source-id: 217e6eb232c323b7b0d50374f3df41e558ab93d4
Summary: In a previous diff, I had changed the name of an old signature of makeTracingHermesRuntime, compiling to make sure it was unused. Then I forgot to delete the declaration, and landed the diff. This diff removes it.
Reviewed By: dulinriley
Differential Revision: D20206249
fbshipit-source-id: c2725a149bf9c04f76cd565103e612e435e9cc21
Summary: Through something of an oversight, previous work had hermes_tracing.h include an LLVM header. This was unnecessary; this diff removes that and the makeHermesRuntime variant that takes an llvm::raw_ostream from the external API. That was not used by anything external, and can be moved into the .cpp file.
Reviewed By: avp
Differential Revision: D20200884
fbshipit-source-id: ca81513afdbc2239a89cf36f535c1550a9bbed7d
Summary: The `Function::call` method says it leaves the JS `this` object undefined. According to my tests, that's not completely true: if the function is defined to use strict mode via `"use strict"` either inside itself or in the file it was defined in, it does leave it `undefined`, but if the function is defined in non-strict mode, it sets `this` to the global object instead. This diff updates the documentation to reflect this.
Reviewed By: mhorowitz
Differential Revision: D19613483
fbshipit-source-id: 4b9ecf81c6318592be05a216748dcb22e32989f8
Summary:
Converting `PseudoHandle` to `Handle` required a `toHandle` function
which didn't follow the pattern for any other Handle creation functions.
Remove `toHandle` and add an overload to `HandleRootOwner::makeHandle`
which allows for the same functionality.
Reviewed By: tmikov
Differential Revision: D19437464
fbshipit-source-id: 40cbab42fd3f008fd319160cf4dd2e0a8c886e64
Summary: The -track-io option was broken in two separate ways(!) Firstly, the printing of results had gone missing in a refactoring. Secondly, a check for non-bytecode input to the trace interpreter was missing the "non-" part.
Reviewed By: dulinriley
Differential Revision: D20038011
fbshipit-source-id: d650efa866d0ba76cd73e1772336618116b6d355
Summary:
The `StorageProvider` used to require being specially deleted after the `Runtime` in order
to ensure that the Runtime itself could be destructed.
Since compressed pointers doesn't require this feature anymore, delete the special logic
from the `Runtime::create` function.
Some classes relied on the `StorageProvider` always outliving the Runtime and only took
raw pointers. Have these take `shared_ptr` instead to make sure the provider dies after
everything is destructed.
Reviewed By: davedets
Differential Revision: D19705134
fbshipit-source-id: fa458a5520d2146ea6174bebf0fcf78ef0ec9556
Summary:
In all other areas of JSI, std::string is treated as potentially
containing UTF-8 bytes (instead of ASCII). This fixes the inconsistency.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D19871520
fbshipit-source-id: c703f07e10bedbf2518d0bec903f85f43bbcbdf5
Summary:
This diff does the major work of making synth traces incremental. The largest part of a trace is it's "trace" component, the array of trace records. This can be quite large -- many thousands of records in a big trace. We "chunk" these: we break the "trace" key into as many "trace_N" keys as necessary, each holding up to some constant (100, in this diff) number of records. We accumulate records until we reach the chunk size, write the records to the trace as the next "trace_N" component, and start accumulating records again.
The components that read the trace -- the SynthTraceParser, and the python script that gathers the source files, etc. -- are adjusted to rebuild the initial "trace" sequence of records before processing.
(The plan is to continue this chunking strategy with other unbounded arrays, like the sequence of results of Date-related calls. But the trace records is by far the largest concern.)
Reviewed By: dulinriley
Differential Revision: D19610511
fbshipit-source-id: 5210ed87592dfa3cc1b35b889284b7c038064e6c
Summary: As part of making traces incremental, this diff moves the recording of the GCConfig information to the SynthTrace constructor (rather than recording it at the end).
Reviewed By: haozhun, dulinriley
Differential Revision: D19610257
fbshipit-source-id: 997ffc7b9ce5dd0f4e292606db7fd8f393109340
Summary:
If we're using synth traces in production, we probably don't want to accumulate the trace in memory, and write it to a file in one big step when an error happens. Rather, we'd like to write the trace incrementally, to a file: file system space is less scarse than memory, and doing it incrementally means less to do in the error handler.
This diff is the first step towards writing the synth trace incrementally. The most difficult aspect of this is that in the existing code, on Android, the trace is written in the handler of an Intent. Java-level operations on the Intent yielded a temporary filename under the application (so if the application is uninstalled, these temporary files are deleted). We must do something similar, but choose the filename within native code, on construction of the SynthTrace object. We copy what profiling does for this: assume the tmp dir is /data/data/<app>, where <app> can be found from reading /proc/self/cmdLine.
The SynthTrace constructor now takes a new argument: a unique_pointer to a stream. If null, no trace file is written, and the SynthTrace just accumulates records in memory. (This functionality is used during trace replay.) . If non-null, the trace is written to that stream.
The "write{BridgeTraffic}Trace..." methods become "flushAndDisable{BridgeTrafficTrace}" methods.
This diff is the first step towards incremental traces: the trace is still written at the end, but we've arranged that the place to write it is available at construction of the SynthTrace object. Later diffs will move recording of static things (e.g., the RuntimeConfig) into the ctor, then actually make us write the trace to the file incrementally.
For ReactNative:
Changelog: [Internal]
Reviewed By: haozhun, dulinriley
Differential Revision: D19471297
fbshipit-source-id: c1de4d2d9f44a87c7ff6fea38a1ce67de593940c
Summary:
Synth trace replay started crashing recently. This turned out to be due to a source of non-determinism we had noticed previously. See the test plan for why.
This diff treats the results of HermesInternal.getInstrumentedStats() as we do, for example, Date.now(): we log the results during the original execution, save these logs as part of the trace, and substitute the logged results for the current results on replay.
Reviewed By: dulinriley
Differential Revision: D19844732
fbshipit-source-id: ecd43d3594dbb3ebab4a501ceea022dfd600c927
Summary:
Instead of returning a `bool` which gives no information about the cause of the error,
return `void` and throw when there's some error.
Another alternative is returning `std::error_code`, but that's less flexible than throwing, and
this API already supports throwing.
Changelog: [Internal]
Reviewed By: jbower-fb
Differential Revision: D19170033
fbshipit-source-id: 870cd996a1a53c94524455f31765c1da99f57a1d
Summary:
Place a fork of LLVM git rev c179d7b006348005d2da228aed4c3c251590baa3
under `external/llvh`(1) and modify the CMake build to use it.
I am calling it a "fork", because it is not all of LLVM. Only the parts
that are used by Hermes are included, which at this time is only parts
of `libLLVMSupporrt`. Most(2) of LLVM build scripts are removed, and it
is treated just as another ordinary library.
(1) Why `llvh`? To be able to coexist with instances of the "real" LLVM,
we must change the namespace, all public symbols containing the `llvm`
string and the include directory name. `llvh` seemed as good a name as
any. I also considered `llvm-h` and `h-llvm`, but the problem is that
they are a superstring of `llvm` so it becomes harder to search for the
`llvm` string.
Note that the actual rename will happen in a follow up diff. It would be
a massive patch.
(2) libLLVMSupport relies on pretty elaborate feature detection scripts,
which would be painful to duplicate, so for now I have preserved them
under external/llvh/cmake.
Unfortunately turning LLVM into an ordinary library is not enough, since
we were implicitly relying on a lot of functionality provided by the
LLVM build scripts. Things like setting default warning flags, easily
turning exceptions on and off, etc.
I attempted to replace it with Hermes equivalents, which are now
provided by `cmake/Hermes.cmake`:
- `add_llvm_library/tool()` is replaced by `add_hermes_library/tool()`.
- Several `LLVM_xxx` variables are replaced my similar `HERMES_xxx`
ones.
As a result, building Hermes now requires only checking it out, and
running CMake and Ninja. It is a vastly simpler process than before.
== Limitations
- CMake LTO and ASAN builds aren't supported yet.
- The JIT requires the "real" LLVM for disassembly.
Reviewed By: avp
Differential Revision: D19658656
fbshipit-source-id: 5094d2af45e343973b1aab02c550a18b2bf93a06
Summary:
Adds a new method `HermesRuntime::getIOTrackingInfoJSON()` which returns JSON data like this:
```
[
{
"url": "file.hbc",
"tracking_info": {
"page_size": 4096,
"total_pages": 30,
"accessed_pages": 22,
"page_ids": [0,21,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20],
"micros": [0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,1,1,0,1]
}
}
]
```
In this data "page_ids" can be turned into HBC byte-offsets by multiplying by "page_size".
This data can be used in combination with data from hbcdump from the HBC file to determine application-level content of these pages (e.g. function names).
Reviewed By: kodafb
Differential Revision: D19349714
fbshipit-source-id: 3739fb9fbcda396f5df3f8d6100f512f63c1fe03
Summary:
The documentation in two out of the three variants of `Function::callWithThis` incorrectly stated that the `this` value was to be `undefined` instead of set to `jsThis`, which contradicts the point of the `callWithThis` method existing in the first place. This diff fixes that issue.
Changelog: [General] [Fixed] - Fix documentation comments for HermesJS's `Function::callWithThis` method to accurately reflect how `this` is handled.
Reviewed By: jbower-fb
Differential Revision: D19714074
fbshipit-source-id: 123688c1f7e578d6356bec54604fb6e30f06b0b1
Summary:
This change will keep project consistency.
This change repeat the previous commit 582738bdc8ffd391362faf8b75ff17c1b024e2c3.
That was created by sammy-SC and reviewed by shergin
That changed
* `ASSERT_TRUE` -> `EXPECT_TRUE`
* `ASSERT_NEAR` -> `EXPECT_NEAR`
* `ASSERT_EQ` -> `EXPECT_EQ`
That said
> 1. Replace ASSERT_* with EXPECT_*. Assert is a fatal assertion. Expect is non-fatal assertion. So if assert fails, tests do not continue and therefore provide less information.
>
> 2. Rename tests in `RawPropsTest.cpp` from `ShadowNodeTest` to `RawPropsTest`.
>
> Source: https://github.com/google/googletest/blob/master/googletest/docs/primer.md#basic-assertions
## Changelog
[CATEGORY] [TYPE] - Message
Pull Request resolved: https://github.com/facebook/react-native/pull/27850
Differential Revision: D19568014
Pulled By: shergin
fbshipit-source-id: 7c22cd1c7ec919675e834a060bd5e681d43a8baf
Summary:
This fixes a crash caused by a `JSError` exception not being successfully caught in a different dynamic library from where it was thrown. Since the libraries were compiled with hidden symbols and loaded with `RTLD_LOCAL`, the exception typeinfo becomes unique to each module.
Reading on this subject:
https://gcc.gnu.org/wiki/Visibilityhttps://stackoverflow.com/questions/14268736/symbol-visibility-exceptions-runtime-error
Reviewed By: mhorowitz
Differential Revision: D19343161
fbshipit-source-id: 4eb3bc2576bbcca2c3aef4f52b5a27dfde214c6a
Summary:
When asking for the property names of a host object (for example, when doing `for (var prop in hostObj)`),
it wasn't being traced. This meant that HostObjects didn't report the same property names before and
after tracing.
This diff implements support for these functions, which require similar support to other HostObject
magic goodies. It requires tracking the entrance and exit of the native code, in case it recurses back
into JS. The mechanism is shared with all other native call transitions.
Reviewed By: davedets
Differential Revision: D19134867
fbshipit-source-id: dae04dcad1251fb4b4ced20040603a513de64286
Summary:
`HostFunction` should also track its name and number of parameters at the time of its
creation, just in case something relies on that behavior in the wild.
The trace format is backwards-compatible, and will still parse and exec existing traces.
The default name if none is specified is the empty string, which will be reported as `"anonymous"`
in various places automatically. The default `paramCount` is 0, which matches what `TraceInterpreter`
did before this change.
Reviewed By: davedets
Differential Revision: D19221486
fbshipit-source-id: 283b6e0a72d9d017f3e4957cb038a7edfc303ee1
Summary: HermesValue::encodeNativeValue was a method for encoding a 47 bit value. This feature is not really used, and it would complicate any implementation of 32-bit HermesValues.
Reviewed By: avp
Differential Revision: D19349163
fbshipit-source-id: 047e3052fff93f82e8f0da208ee076d40167631a
Summary: This actually installs Proxy on the global so it can be used.
Reviewed By: avp
Differential Revision: D18284934
fbshipit-source-id: 33c457cd45afe7f0e41f8b2c2d688654d2093d5d
Summary: Recent experience has made it clear that we really ought to determine whether mark stack overflows are happening in the wild. This diff enabled that (I hope).
Reviewed By: dulinriley
Differential Revision: D19201791
fbshipit-source-id: 215543e33a98890507fc20d2b9bca1dd5464d961
Summary:
The `GCConfig` was being copied for `StackRuntime`, but it no longer needed that config
(it went back to using `mmapProvider` a while ago).
This copy was problematic because it ran a destructor on the `runtimeMemoryThread`.
One of the fields of the config was a `std::function` that happened to hold a `jni::global_ref`.
Those references can't be destructed on a thread not registered with the JVM, which
`runtimeMemoryThread` wasn't.
Reviewed By: mhorowitz
Differential Revision: D19277414
fbshipit-source-id: 0c12c9590132fee1ba9a841a97d656da6b1b5b06
Summary: This data is not reported anymore.
Reviewed By: dulinriley
Differential Revision: D18904902
fbshipit-source-id: 4a6f018722fbf9c0ac290b4504dc14b69b5de8c6
Summary:
If building a JSError causes a JSError, this would lead to infinite recursion. This changes the error handling path so the problem is visible to JS. There's also a few related cleanups included, and a new test case.
Changelog: [General] [Fixed] - If Error global is not callable when building an error, jsi will throw a JS exception back to JS code. #158
Reviewed By: tmikov, dulinriley
Differential Revision: D18796269
fbshipit-source-id: 57a45d144fa2ea5e78d18c27d3130611737dda96
Summary:
Now an Error message will look like
```
Exception in HostObject::get: for prop 'noprop': <unknown exception>
```
Reviewed By: dulinriley
Differential Revision: D18365598
fbshipit-source-id: 8bc8bc440fb77c28fe3048265bcd6e37cd6937e1
Summary:
Before, if the top-level `TraceInterpreter::exec` was called with an empty vector,
it would print:
```
Provided source hashes:
```
Instead, have it print:
```
Provided source hashes: (no sources provided)
```
To make it more obvious to the user what went wrong.
Reviewed By: kodafb
Differential Revision: D19180522
fbshipit-source-id: 5a04b894e867858365a4d2366852a2beb8aaebd7
Summary:
In case `HostFunction` or `HostObject` functions throw, crash early instead of letting the exception propagate
through JS.
If JS were to catch the exception and try to call other Host-defined things, the original error can get lost.
Reviewed By: kodafb
Differential Revision: D18863025
fbshipit-source-id: 70ab5cc483541a88604a5723fd6818c9154454fb
Summary:
The `EndExecJS` event already emits a marker automatically, so don't repeat it in the `Marker` event
case (which `EndExecJS` falls through to).
This only affected re-traced benchmarks (when you turn tracing on during replay), so this bug and fix has
no affect on just running the benchmarks normally.
Reviewed By: kodafb
Differential Revision: D18863026
fbshipit-source-id: c67aef6c4008236d4a72df66eb495e1c4e4debd0
Summary:
Hermes currently has a number of warnings and one error when building in gcc9.
This fixes most of the warnings and the error.
Pull Request resolved: https://github.com/facebook/hermes/pull/161
Reviewed By: tmikov
Differential Revision: D18823794
Pulled By: mhorowitz
fbshipit-source-id: d4a4555737151325696d608afa2d7af0b2e5cdeb
Summary:
`JSONParser` would store a string that had surrogate pairs as UTF-8 with
unconverted surrogates. This meant that using the characters as UTF-8 was mostly fine,
until a surrogate pair was found.
The fix for this is to add a mode flag to have JSLexer fix the encoding of these, by converting to UTF-16 and
back to UTF-8, fixing the surrogates while that happens.
This mode flag defaults to `false`, and only synth traces care about it right now.
This change should only minimally affect the normal parser performance.
Reviewed By: tmikov
Differential Revision: D18722155
fbshipit-source-id: a9fc140d77d223a872310efae7edadeb79af3711
Summary:
`sourceURL` is a parameter passed into `evaluateJavaScript` that wasn't being tracked before.
This results in some `Error` stack traces not aligning between the original run and replays.
So far this hasn't resulted in a behavior change, but it's always good to be cautious with synth
benchmarks.
Reviewed By: kodafb
Differential Revision: D18722154
fbshipit-source-id: 5349a81076843725ed0babf744921803fdb0aaa4
Summary:
Hermes's standard is to have private members have trailing underscores.
I had forgotten to do that originally in the `TraceInterpreter`, and this was inconsistent
with other parts of Hermes.
There's no semantic difference from this change, just a refactor.
Reviewed By: tmikov
Differential Revision: D18729830
fbshipit-source-id: 13ba7a7cf792f6304a9f9769a51c9787531a6d5f
Summary:
If opcode profiler is turned on while running synth benchmarks, there's a compiler error due to type mismatch.
This change adds a dynamic cast of JSI Runtime to Hermes Runtime so opcode profiler would work.
Reviewed By: dulinriley
Differential Revision: D18642058
fbshipit-source-id: b987d297d4bb3881433007d3a371d10c36f97270
Summary:
Sometimes multiple bytecode files are processed by Hermes in one execution, and this previously
would break synth tracing because it assumed only one file would be used.
Extend the system to allow multiple `BeginExecJS` events on different files.
File identities are tracked via a `sourceHash` which is a SHA-1 hash of the source file,
either computed from the source or extracted from the bytecode (where the compiler embeds the source
hash).
When the trace is taken, the sourceHash is recorded for each file that was executed. Upon replay, a set of
files with the same set of source hashes are expected. The source hash is used as a mapping key in the
implementation to associate a file given on a command line to a specific set of `BeginExecJS` calls.
As an escape hatch for the common case and backwards compatibility, if the sourceHash isn't present in a
BeginExecJS record (or is all zero), that is assumed to match a single file.
NOTE: `std::map` was used because there is no `std::hash` implementation provided for `std::array`.
Reviewed By: kodafb
Differential Revision: D18088907
fbshipit-source-id: 945e1c509eada5bae7a6e017ed67d82e5fbde079
Summary:
These extra flags will make it very clear to both readers and to the implementation
what is supposed to happen with this synth trace file.
Reviewed By: davedets
Differential Revision: D18234638
fbshipit-source-id: 343601f4be197041efa13fed882694bee4e5c066
Summary:
There's no real need to conserve disk space when using traces, as it's only used
in debugging/local scenarios. The niceness of opening the trace file with vim is much
appreciated over the compactness of the file.
So turn on `pretty` mode by default whenever writing out a trace file.
Reviewed By: davedets
Differential Revision: D18234636
fbshipit-source-id: ccb501eb6137930ffc2c52e9ec4b9fc1de92ac02
Summary:
This parameter used to be useful for a custom format hermes was developing,
but since Hermes now outputs the Chrome format it isn't useful.
Chrome actually disallows prettified JSON, and requires a special version that is faster to parse.
Therefore, `compact` was the only supported mode.
Changelog: [Internal] Remove compact parameter from `createSnapshotToFile`
Reviewed By: willholen
Differential Revision: D17726742
fbshipit-source-id: 6f39af9046dff2f3b4fba822312a9a89c939ed89
Summary:
eval() does not allow lazy compilation, while most other mechanisms do.
This can cause unnecessary memory usage and loading time when using
eval() to inject large bundles. This change allows eval() to use the
same mechanism as JSI prepareJavaScript, and reuses its lazy compilation
support.
Reviewed By: tmikov
Differential Revision: D18237407
fbshipit-source-id: 23858d06ffef4beb6ec4fe955cd0cf101a115365
Summary:
Use UTF16Stream when parsing JSON. This avoids converting the entire input upfront, reducing peak memory usage for large inputs.
Perform some slight required refactoring in JSONLexer to conform to the stream API: save token character instead of token position (for error reporting), avoid prefix ++, and tweak number parsing to not save stream positions.
Reviewed By: mhorowitz
Differential Revision: D17726475
fbshipit-source-id: 95ad8dfebad7e2d8d769acc63b7761ce56e3380e
Summary:
In order to use tracing, several files must be compiled into the API of Hermes.
This can bloat the binary by several kilobytes. Add a build mode to allow it to be compiled
out. Off by default.
Reviewed By: jbower-fb
Differential Revision: D17982482
fbshipit-source-id: 36fb2d90963cacd5d7fd6152af502d98e406b6e9
Summary: Add support for overriding the default initial heap size for the GC when running a synth benchmark.
Reviewed By: kodafb
Differential Revision: D17981615
fbshipit-source-id: 95342fa365854f1b7c318012c264dafe5b5d6fc9
Summary:
Sometime more than one bytecode or source file is loaded and executed in Hermes while tracing occurs.
We should support allowing multiple BeginExecJS events by moving the source hash check there.
This diff only moves where the `sourceHash` is located, and doesn't yet handle multiple bytecode files.
Reviewed By: kodafb
Differential Revision: D17584085
fbshipit-source-id: 057b62bae3e7d897e52f263e177598b629903ec8
Summary:
When attempting to step over certain statements such as `console.log`,
the inspector can attempt to stop the debugger in order to print things,
inspect variables, etc.
This breaks an invariant in the debugger that we do not clear temporary
stepping breakpoints unless we have completed the step.
We cannot avoid this issue by ignoring async break requests during stepping,
because the user could explicitly request a break in an infinite loop
during a step over/out, for example.
To deal with this, we require API users to specify whether a pause is
implicit or explicit, and we pass that information through from the inspector.
We then have two bits in the async atomic, and we let the debugger know
on pause which kind of pause was requested.
If the pause was implicit, then we ignore it if we are currently stepping
because we'll likely pause soon.
To handle the infinite loop case, if the pause was explicit, we
clear any step state and do a hard pause as if the current point was a
breakpoint. Because any step state has been cleared, we're good to
perform more debugger actions as requested by the user.
Reviewed By: willholen
Differential Revision: D17857679
fbshipit-source-id: aff69c79e8ef3d9efaf1f1c2d25bdc938734a766
Summary:
Add a (hidden) option `-stable-instruction-count` to be used for testing only. It makes the VM try hard to execute the same number of CPU instructions across repeated invocations of the same JS. It currently does this by silencing stats logging (since printing time measurements is problematic), and fixing the seed for Math.random. It does *not* handle Date, which is still a possible source of non-determinism, unless it's mocked as part of a synthetic trace.
One use case is to track down which change, from a large stack or time interval, affects instruction counts more than expected.
Reviewed By: dulinriley
Differential Revision: D17907769
fbshipit-source-id: d2ca6d5943ad5396753d94a9072e405c1035d495
Summary:
This error message is out of date, and that build flag doesn't exist anymore.
The new way to take synth traces is with `makeTracingHermesRuntime`.
Reviewed By: willholen
Differential Revision: D17881294
fbshipit-source-id: 721302c178bf4617547136a0e86ce26b030444bc
Summary:
D17833692 introduced tracking the maximum of used size before/after after GC, and exposed this data as "peakAllocatedBytes" and "peakLiveAfterGC".
The latter was incorrectly calculated for the generational GC. Measuring "live" data only makes sense after a *full* GC, but the code was looking at GCBase::cumStats_ which contains an indiscriminate mix of young and full collections. (This is in contrast with "peakAllocatedBytes", which can legitimately occur before any kind of collection, hence GCBase::cumStats_ is correct there.)
Fix the issue by refactoring the code to allow overriding peakLiveAfterGC for GenGC.
Reviewed By: davedets
Differential Revision: D17847598
fbshipit-source-id: 605a40050ac3801b8fd36c2b8e1817f5f830c5df
Summary:
The API CMakeLists was using compile flags that don't exist on Windows.
Add a branch to choose the correct flags.
Also use `CXX_STANDARD` to select a standard without using compiler flags.
Reviewed By: willholen
Differential Revision: D17833866
fbshipit-source-id: 1a2f14133f8f3e415b0ac26219c1b8661be93464
Summary: Add heap usage before/after GC into the cumulative stats. Also report the max across time for these as "peakAllocatedBytes" and "peakLiveAfterGC" in the printed GC stats and the getHeapInfo API.
Reviewed By: mhorowitz
Differential Revision: D17833692
fbshipit-source-id: bce7de5818ad1aea6b7025d74cec8cd8e118b82a
Summary:
`GCConfig` and `RuntimeConfig` occasionally have important fields that can affect performance of the
benchmark, and need to match what is used in production.
Have SynthTrace emit these, and the parser use those same values (unless overridden with local runtime flags).
This is a backwards compatible change, old traces will get default values. New traces will record what values they
were run with.
Reviewed By: kodafb
Differential Revision: D17643344
fbshipit-source-id: 2ddab7dc736a4a49e35e6522d0d6efdd9b8692f2
Summary:
Extend the option to release unused memory from a boolean to an enum, adding levels that also release young gen memory, either only on full collections, or also on young gen collections. Increasing the level has some performance cost.
Default behavior is unchanged.
Reviewed By: davedets
Differential Revision: D17609356
fbshipit-source-id: 065a329a6fed1d80668cf5035bc5d3ce77506d10
Summary: The default implementation of jsi::Runtime::createValueFromJsonUtf8 creates a temporary string in the JS heap which becomes garbage immediately after parsing. The new implementation avoids this JS heap allocation and immediately frees the memory after parsing.
Reviewed By: tmikov
Differential Revision: D17638279
fbshipit-source-id: 5b82ba1d73ad44c668da092fa9ac74ae5bf7ce8f
Summary: Make it possible for a Runtime implementation to provide its own JSON parsing, by making the method on Value call through Runtime, which now has the default implementation.
Reviewed By: tmikov
Differential Revision: D17637395
fbshipit-source-id: b8997f7d1721a7790326417f3abfa86c875923c9
Summary:
A major parameter of the heap growth policy is the occupancy target - the target fraction of live data in the heap. A higher value typically means more conservative growth and thus also more frequent collections.
Expose this as a configurable parameter (both in RuntimeConfig API and in the command-line runners).
Reviewed By: davedets
Differential Revision: D17604669
fbshipit-source-id: 1160ff83af1b8d188a799ca943a79f64dba60eb4
Summary:
If a version isn't specified, the version check is avoided in synth traces. It mostly exists to prevent
accidental errors in published benchmarks.
The version isn't important to these tests, and every time I update the version I don't want
to have to update a bunch of tests.
Reviewed By: jbower-fb
Differential Revision: D17584567
fbshipit-source-id: 6467ee7d666a0ce7d9795f7181ac28995a6f6d70
Summary:
Now that tracing is available without a build mode, make replay also available without a build mode,
as it uses all the same mechanisms.
Reviewed By: jbower-fb
Differential Revision: D17053870
fbshipit-source-id: 92a12fc281ccbb9929f8db0a53156a17245bf428
Neil Dhar
Anandraj
Thibault Malbranche
Daniel Andersson
Riley Dulin
Xuan Huang
Gijs Weterings
Tzvetan Mikov
Haochen Li
Andres Suarez
Eloy Durán
Anandraj
Marc Horowitz
Aakash Patel
grabbou
Will Holen
Martin Sherburn
Ze Tian
Christoph Purrer
Pieter De Baets
David Detlefs
Peter Ammon
Ryan Tremblay
Haozhun Jin
Jeffrey Tan
Jacob Bower
Gabriel Nunes
Yuki Ueda
Scott Kyle
Michał Gregorczyk
Kiwi Chen