GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — V8 v11 prep by edusperoni · Pull Request #1794 · NativeScript/android

Conversation

@edusperoni

@edusperoni edusperoni commented Nov 28, 2023

Copy link
Copy Markdown
Collaborator

This is the prep work for v8 version 11+. Creating the PR so we track it more easily

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
ptomato and others added 23 commits September 22, 2023 18:28
@ptomato @edusperoni
chore: Use jint for Java object ID everywhere
a96d750
The Java object ID is an integer returned from Java, so it should be typed
as jint. jint is int32_t, so using uint32_t is incorrect here. Using int
is probably OK, but change it anyway for consistency.
@ptomato @edusperoni
chore: Avoid shadowing variable
9c53587
Shadowing the outer jsInfo variable looks like a bug and causes a compiler
warning. But it actually is not a bug, because the earlier
GetJSInstanceInfo() call returns the JSInstanceInfo* pointer, whereas we
need the v8::External to put into the internal field. It's easier to reuse
the same External than to create a new one.
@ptomato @edusperoni
chore: Remove dead code from ObjectManager
f2d6cac
These may be left over from previous refactorings. There are some methods
of ObjectManager that are never used, some pieces of data that are stored
but never read, and some fields that are never stored in the first place.
@ptomato @edusperoni
feat: Remove markingMode
29311b2
BREAKING CHANGE: The "markingMode" configuration option in package.json
no longer has any effect. It is always set to "none", which was already
the default. "markingMode": "full" will be ignored.

BREAKING CHANGE: The `__markingMode` global variable no longer exists.

Removing support for full marking mode allows removing a big chunk of GC
finalizer code, which would otherwise need to be ported to CPPGC.
@ptomato @edusperoni
chore: Remove useGlobalRefs
8d5f8f5
This is always set to true for SDK level 16 and later; and we require
level 17 to build. So it doesn't need to be configurable.
@ptomato @edusperoni
chore: Init ObjectManager::m_isolate in Init()
4ec8502
Remove the ObjectManager::SetInstanceIsolate method. It can be considered
part of initializing the ObjectManager after the Isolate is created.
Previously, both SetInstanceIsolate and Init were called with the newly
created Isolate in Runtime::PrepareV8Runtime, but we don't need to have a
2-stage init process for the ObjectManager.
@ptomato @edusperoni
chore: Initialize ObjectManager in one phase
d7a590d
Combine the ObjectManager::Init() method into the ObjectManager
constructor. This way, we don't have to worry about the isolate and things
that depend on it being null. Give Runtime a unique_ptr<ObjectManager>
member.

I'm planning to use this in the upcoming CPPGC port, where I'll be
defining more function templates in the constructor.
@ptomato @edusperoni
chore: Make some methods static
489100c
These methods don't use the instance pointer, so they can be static - at
least according to Android Studio's static analysis.
@ptomato @edusperoni
chore: Store wrapper object template instead of ctor function template
cc6fd5d
Previously we stored a function template in ObjectManager, with which a
constructor function could be created, which would return an empty wrapper
object. Using an object template should achieve the same effect, but
without a call back into JS, so slightly more efficient.
@ptomato @edusperoni
fix: NativeScript include directories in cmake file
917422f
The previous cmake code accidentally set the INTERFACE_INCLUDE_DIRECTORIES
property to the string "NATIVES_BLOB_INCLUDE_DIRECTORIES". We actually
want the value of the variable. (I don't think this affects anything in
the current build, but it complains that INTERFACE_INCLUDE_DIRECTORIES
contains a relative path if you try to link anything else against
libNativeScript.so.)
@ptomato @edusperoni
fix: passing wrong arguments to NativeScriptException ctor
e8e9dec
In the case where a Java exception is thrown from a JNI call, wrapped into
a C++ NativeScriptException, and caught by C++ code that re-throws it to
Java, we'd get a crash.

(Admittedly, this does not happen very much. Initializing the
ObjectManager is one of the few places where it can happen.)
@ptomato @edusperoni
fix: Populate keywords set during static initialization
016c077
Running the worker tests would occasionally hit a crash where the keywords
set contained corrupt data. I believe this is because the keywords.empty()
check was being hit in two different threads. We can supply the keywords
in an initializer list so that the set is fully populated during static
initialization.
@ptomato @edusperoni
fix: init NativeScriptException earlier
e87074a
Unlikely, but the ArgConverter calls at the beginning of Runtime::Init()
could theoretically throw a Java exception, in which case they'd be
wrapped into a C++ NativeScriptException and then re-thrown to Java. For
that to work, NativeScriptException has to be initialized.
@ptomato @edusperoni
fix: move GetClassName out of ObjectManager
e284c07
Particularly in NativeScriptException::ReThrowToJava(), we may need to
call GetClassName before the ObjectManager is initialized. This would
cause a crash when handling exceptions early in the NativeScript runtime's
initialization process.

Luckily, GetClassName doesn't actually need to be part of ObjectManager,
it can be moved to JEnv instead.
@ptomato @edusperoni
fix: Safety check on clearing dex cache
91ca870
Giving a wrong path to DexFactory can result in folders unexpectedly being
wiped. Add a safety check to DexFactory.purgeDexesByThumb() to make sure
we are only deleting files with the expected extensions.
@ptomato @edusperoni
chore: Switch from CreationContext() to GetCreationContextChecked()
c58d3ff
See v8/v8@b38bf5b0. The creation context may be
null for WASM objects, so the old API is deprecated in favour of
GetCreationContext() which returns a MaybeLocal<Context>. However, we are
not handling any WASM objects here, so we can use the version of the API
that asserts that we do have a creation context.
@ptomato @edusperoni
chore: Set V8 flags before initializing V8
492dadc
In 11.x, it's no longer allowed to change the flags after V8 has been
initialized.

We can also avoid storing the string in Constants::V8_STARTUP_FLAGS, as
it's no longer used anywhere else in the code base.
@ptomato @edusperoni
chore: Use CPPGC-compatible platform
9a96029
For the CPPGC port we'll need to use a cppgc::DefaultPlatform instead of
the v8::platform::NewDefaultPlatform(). It's also harmless to use it
already even if we don't use CPPGC yet.
@ptomato @edusperoni
chore: Remove unused V8 inspector sources
a11a490
These sources are already compiled into the V8 artifacts, and are not
compiled as part of the runtime.
@ptomato @edusperoni
chore: Remove duplicate code
34d2786
This line shadows a v8::Context already present in the outer scope, so
it's not necessary.
@ptomato @edusperoni
chore: Make MetadataNode::tryGetExtensionMethodCallbackData more effi…
18f90c4
…cient

Two changes suggested by Android Studio. We avoid copying the string
argument, and we check quicker whether the map is empty without having to
count all the entries.
@ptomato @edusperoni
chore: Remove ObjectManager dead code
1843dd2
JSWrapperConstructorCallback isn't used anywhere.
@ptomato @edusperoni
chore: Avoid copy in MetadataNode::IsJavascriptKeyword
25338bf
Noticed while fixing static initialization of keywords set, see next
commit.
@cla-bot cla-bot Bot added the cla: yes label Nov 28, 2023

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@edusperoni @ptomato
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.