[ammarahm-ed]

Port of the MetadataNode part of NativeScript/android#2013, in both the Node-API and the direct runtime.

s_name2NodeCache, s_name2TreeNodeCache and s_treeNode2NodeCache are process-wide maps. Every runtime thread, including each worker, reads and inserts into them when it wraps a Java object, and nothing guarded them. A concurrent insert that rehashes a robin_hood map while another thread reads it is a data race that can crash or return a stale node.

They are now guarded by one s_nodeCacheMutex, held only for the map access and never across the metadata reader or a MetadataNode constructor. When two threads resolve the same class at once, both may build a node and the first one to insert wins; the loser is deleted before it is published. GetOrCreateTreeNodeByName, GetOrCreate and GetOrCreateInternal are rewritten around that, with the same results as before for a single thread.

This does not touch MetadataReader, which mutates the metadata tree while calling back into Java and needs its own design (see below).

Results

V8 release build on a Sony SO-01M (Android 11, arm64), AOT-compiled with cmd package compile -m speed. One warm-up launch is discarded, then 5 launches; values are the median ns per call. This is a correctness fix, so the number to watch is that the hot path does not get slower.

Case	Node-API before	after	Δ	Direct before	after	Δ
Return a new object	13,293	12,793	-4%	13,295	13,176	-1%
Call a method on a new object	20,101	19,006	-5%	20,254	20,442	+1%
Return a Date Object	14,240	13,597	-5%	14,413	14,660	+2%
Return this	1,380	1,254	-9%	1,261	1,246	-1%
Void Method on instance	653	650	-0%	582	577	-1%

Across all rows the median change is -1.0% on Node-API and -0.3% on the direct runtime, so the lock is not measurable here; individual rows move by up to about ±10%, which is this device's noise.

All cases

Case	Node-API before	after	Δ	Direct before	after	Δ
Void Method on instance	653	650	-0%	582	577	-1%
Field on instance	876	838	-4%	876	860	-2%
Int Field on instance	377	372	-1%	396	428	+8%
Get Static Field	804	856	+7%	844	837	-1%
Void Static Method	305	301	-1%	316	308	-2%
multiply(a, b)	457	464	+1%	531	559	+5%
Pass and return string	1,180	1,180	+0%	1,242	1,099	-11%
Return a String	795	785	-1%	796	764	-4%
Return an Int	319	319	+0%	313	315	+1%
Pass a String	664	621	-6%	695	649	-7%
Return a Boolean	321	322	+0%	310	312	+0%
Return a Double	389	387	-0%	395	370	-6%
Pass an Int	376	372	-1%	424	453	+7%
Pass a Double	372	365	-2%	439	455	+4%
Pass a Boolean	457	457	-0%	434	417	-4%
Return an Int Array	29,192	28,177	-3%	20,812	20,749	-0%
Pass an Int Array	2,757	2,793	+1%	3,615	3,658	+1%
Return a String Array	30,443	29,619	-3%	21,765	21,710	-0%
Pass a String Array	5,429	5,635	+4%	6,630	6,829	+3%
Return a Double Array	28,644	27,757	-3%	21,309	21,197	-1%
Pass a Double Array	2,843	2,820	-1%	3,600	3,758	+4%
Return a Boolean Array	27,600	26,792	-3%	20,574	20,247	-2%
Pass a Boolean Array	2,981	3,062	+3%	3,693	3,850	+4%
Int Array[0] read	354	350	-1%	241	240	-0%
String Array[0] read	803	791	-1%	691	701	+1%
Double Array[0] read	482	486	+1%	340	342	+1%
Boolean Array[0] read	349	342	-2%	239	234	-2%
Int Array[0] write	460	467	+1%	379	377	-1%
String Array[0] write	1,171	1,190	+2%	1,069	1,084	+1%
Double Array[0] write	460	469	+2%	379	380	+1%
Boolean Array[0] write	480	477	-1%	383	389	+2%
Return a Date Object	14,240	13,597	-5%	14,413	14,660	+2%
Pass a Date Object	3,795	3,829	+1%	3,604	3,785	+5%
Invoke void callback	7,667	7,458	-3%	6,735	6,690	-1%
Instance add(a, b)	818	808	-1%	828	801	-3%
Pass and return non-ASCII string	1,170	1,175	+0%	1,240	1,101	-11%
Pass and return 1KB string	5,060	4,957	-2%	5,058	4,976	-2%
Return a Char	366	359	-2%	347	348	+0%
Return this	1,380	1,254	-9%	1,261	1,246	-1%
Return a new object	13,293	12,793	-4%	13,295	13,176	-1%
Call a method on a new object	20,101	19,006	-5%	20,254	20,442	+1%
Pass a 16 Int Array	2,791	2,822	+1%	3,678	3,657	-1%
Pass a 16 Double Array	3,356	3,401	+1%	4,176	4,294	+3%
Invoke int callback	13,958	10,957	-21%	10,115	12,922	+28%
Invoke string callback	9,118	10,683	+17%	8,422	8,314	-1%
Native heap after run (MB)	106 MB	117 MB	+10%	105 MB	97 MB	-8%

I could not reproduce the race on demand, so there is no before/after crash rate; the change rests on the maps being shared across threads without a lock. The Jasmine suite has not been run on this branch on its own; the stack-top result is in the PR above it.

Not ported from #2013

• MetadataReader locking and the GetNodeById bounds check, which need a lock that can be released around the Java callback in GetOrCreateTreeNodeByName.
• The main-runtime election and the unwind of a failed init, which are written around V8 isolate setup.

Only V8 was built and run.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c4050155-b57c-437a-b30d-1ca33373dee8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn