Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bridge): Provide automatic garbage collection triggering #1060

Merged
merged 7 commits into from Jan 17, 2019

Conversation

mbektchiev
Copy link
Contributor

@mbektchiev mbektchiev commented Jan 14, 2019

Introduce settings similar to the ones in {N} Android Runtime for
configuring automatic GC triggering by the runtime. They are
specified in app/package.json as values in the ios section:

  • gcThrottleTime - number of milliseconds which must have passed
    since the last automatic GC in order to trigger one during a call from
    JS to native
  • memoryCheckInterval - number of milliseconds which must have
    passed since the last automatic GC in order to check the memory
    usage on the device and trigger GC if free memory is below the specified
    threshold
  • freeMemoryRatio - a floating-point value from 0 to 1 which sets the
    threshold below which a GC will be triggered when the above check is made.

After implementing the feature we faced a number of different bugs leading to random
crashes after a GC has run. The fixes are included in this PR as separate commits:

  • fix(runtime): Keep JS instances in Strong references after creation
  • fix(bridge): Don't read returned value of a function which threw an exception
  • fix(bridge): Don't read JS properties without obtaining a lock
  • fix(build): Regenerate low-level interpreter after build configuration change
  • fix(bridge): Marshal arguments of async calls in worker thread

PR Checklist

Implements #1035

@mbektchiev mbektchiev self-assigned this Jan 14, 2019
@mbektchiev mbektchiev added this to the 5.2.0 milestone Jan 14, 2019
@mbektchiev mbektchiev force-pushed the bektchiev/auto-gc branch 2 times, most recently from 368d7f7 to b5e6a71 Compare January 16, 2019 10:55
Introduce settings similar to the ones in {N} Android Runtime for
configuring automatic GC triggering by the runtime. They are
specified in `app/package.json` as values in the `ios` section:

* `gcThrottleTime` - number of milliseconds which must have passed
since the last automatic GC in order to trigger one during a call from
JS to native
* `memoryCheckInterval` - number of milliseconds which must have
passed since the last automatic GC in order to check the memory
usage on the device and trigger GC if free memory is below the specified
threshold
* `freeMemoryRatio` - a floating-point value from 0 to 1 which sets the
threshold below which a GC will be triggered when the above check is made.

implements #1035
Runtime's stability is compromised when automatic GC triggering is
set to a very small interval (e.g. 1 ms). It turns out that if a GC passes
between the moment when an object is created and it's assigned to a
variable, it can get destroyed prematurely. In order to avoid it we have to
keep such objects as `Strong` references.
…xception

In such cases the memory holding the result is uninitialized and reading it
could crash the application.
In `TNSArrayAdapter`'s `countByEnumeratingWithState` we need to
obtain a JS engine lock before accessing properties of the JS array(-like)
object.
…n change

Make additional stamp files for LLIntAssembly.h per configuration

It seems like changing the build configuration also needs to regenerate
the low level interpreter. If it's not regenerated we receive a linker error.
Set the automatic triggering period to 5 ms to keep the runtime
and it's tests working correctly with frequent GCs.

If sporadic crashes related to GCs start happening we may set this
value to something significantly lower (e.g. 1 ns = `1e-6` ms)
to instruct the runtime to trigger a new GC on each native call.
This isn't the default value because it significantly degrades
performance.
Temporary ObjectiveC objects which are created during marshalling
are autorelease-d. This means that they are destroyed when the nearest
autorelease pool is drained. Due to the refactoring in this PR, the racing
condition between the JS calling thread for destroying them and the worker
thread using them when performing the method call is won by the JS thread.

To fix this we now marshal the arguments in the worker thread and reject the
promise if something goes wrong instead of doing it synchronously and throwing
an exception.
@mbektchiev mbektchiev merged commit 5491345 into master Jan 17, 2019
@mbektchiev mbektchiev deleted the bektchiev/auto-gc branch January 17, 2019 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants