Skip to content

Commit 60516fd

Browse files
authored
feat: upgrade deno_core (denoland#868)
- use new fast call api which allows statically allocating CFunctionInfo so we don't need to manually deallocate them - remove unneeded v8_version function
1 parent c9509bc commit 60516fd

36 files changed

+978
-834
lines changed

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ deno_ops = { version = "0.179.0", path = "./ops" }
2424
serde_v8 = { version = "0.212.0", path = "./serde_v8" }
2525
deno_core_testing = { path = "./testing" }
2626

27-
v8 = { version = "0.102.0", default-features = false }
27+
v8 = { version = "0.103.0", default-features = false }
2828
deno_ast = { version = "=0.40.0", features = ["transpiling"] }
2929
deno_unsync = "0.4.0"
3030
deno_core_icudata = "0.0.73"

core/extensions.rs

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ use anyhow::Error;
1010
use std::borrow::Cow;
1111
use std::marker::PhantomData;
1212
use std::sync::Arc;
13-
use v8::fast_api::FastFunction;
13+
use v8::fast_api::CFunction;
14+
use v8::fast_api::CFunctionInfo;
15+
use v8::fast_api::Int64Representation;
16+
use v8::fast_api::Type;
1417
use v8::MapFnTo;
1518

1619
#[derive(Clone)]
@@ -168,6 +171,11 @@ pub type GlobalObjectMiddlewareFn =
168171

169172
extern "C" fn noop() {}
170173

174+
const NOOP_FN: CFunction = CFunction::new(
175+
noop as _,
176+
&CFunctionInfo::new(Type::Void.scalar(), &[], Int64Representation::Number),
177+
);
178+
171179
#[derive(Clone, Copy)]
172180
pub struct OpDecl {
173181
pub name: &'static str,
@@ -181,9 +189,9 @@ pub struct OpDecl {
181189
/// The slow dispatch call with metrics enabled. If metrics are enabled, the `v8::Function` is created with this callback.
182190
pub(crate) slow_fn_with_metrics: OpFnRef,
183191
/// The fast dispatch call. If metrics are disabled, the `v8::Function`'s fastcall is created with this callback.
184-
pub(crate) fast_fn: Option<FastFunction>,
192+
pub(crate) fast_fn: Option<CFunction>,
185193
/// The fast dispatch call with metrics enabled. If metrics are enabled, the `v8::Function`'s fastcall is created with this callback.
186-
pub(crate) fast_fn_with_metrics: Option<FastFunction>,
194+
pub(crate) fast_fn_with_metrics: Option<CFunction>,
187195
/// Any metadata associated with this op.
188196
pub metadata: OpMetadata,
189197
}
@@ -200,8 +208,8 @@ impl OpDecl {
200208
no_side_effects: bool,
201209
slow_fn: OpFnRef,
202210
slow_fn_with_metrics: OpFnRef,
203-
fast_fn: Option<FastFunction>,
204-
fast_fn_with_metrics: Option<FastFunction>,
211+
fast_fn: Option<CFunction>,
212+
fast_fn_with_metrics: Option<CFunction>,
205213
metadata: OpMetadata,
206214
) -> Self {
207215
#[allow(deprecated)]
@@ -230,26 +238,8 @@ impl OpDecl {
230238
// ideally we would add a fallback that would throw, but it's unclear
231239
// if disabled op (that throws in JS) would ever get optimized to become
232240
// a fast function.
233-
fast_fn: if self.fast_fn.is_some() {
234-
Some(FastFunction {
235-
args: &[],
236-
function: noop as _,
237-
repr: v8::fast_api::Int64Representation::Number,
238-
return_type: v8::fast_api::CType::Void,
239-
})
240-
} else {
241-
None
242-
},
243-
fast_fn_with_metrics: if self.fast_fn_with_metrics.is_some() {
244-
Some(FastFunction {
245-
args: &[],
246-
function: noop as _,
247-
repr: v8::fast_api::Int64Representation::Number,
248-
return_type: v8::fast_api::CType::Void,
249-
})
250-
} else {
251-
None
252-
},
241+
fast_fn: self.fast_fn.map(|_| NOOP_FN),
242+
fast_fn_with_metrics: self.fast_fn_with_metrics.map(|_| NOOP_FN),
253243
..self
254244
}
255245
}
@@ -265,15 +255,15 @@ impl OpDecl {
265255
}
266256

267257
#[doc(hidden)]
268-
pub const fn fast_fn(&self) -> FastFunction {
258+
pub const fn fast_fn(&self) -> CFunction {
269259
let Some(f) = self.fast_fn else {
270260
panic!("Not a fast function");
271261
};
272262
f
273263
}
274264

275265
#[doc(hidden)]
276-
pub const fn fast_fn_with_metrics(&self) -> FastFunction {
266+
pub const fn fast_fn_with_metrics(&self) -> CFunction {
277267
let Some(f) = self.fast_fn_with_metrics else {
278268
panic!("Not a fast function");
279269
};

core/lib.rs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,6 @@ pub use crate::tasks::V8TaskSpawner;
168168
// Ensure we can use op2 in deno_core without any hackery.
169169
extern crate self as deno_core;
170170

171-
pub fn v8_version() -> &'static str {
172-
v8::V8::get_version()
173-
}
174-
175171
/// An internal module re-exporting functions used by the #[op] (`deno_ops`) macro
176172
#[doc(hidden)]
177173
pub mod _ops {
@@ -241,11 +237,6 @@ mod tests {
241237
assert_eq!(&name[..expected.len()], expected);
242238
}
243239

244-
#[test]
245-
fn test_v8_version() {
246-
assert!(v8_version().len() > 3);
247-
}
248-
249240
// If the deno command is available, we ensure the async stubs are correctly rebuilt.
250241
#[test]
251242
fn test_rebuild_async_stubs() {

core/ops.rs

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,11 @@ use std::cell::RefCell;
1414
use std::cell::UnsafeCell;
1515
use std::ops::Deref;
1616
use std::ops::DerefMut;
17-
use std::ptr::NonNull;
1817
use std::rc::Rc;
1918
use std::sync::atomic::AtomicUsize;
2019
use std::sync::atomic::Ordering;
2120
use std::sync::Arc;
22-
use v8::fast_api::CFunctionInfo;
23-
use v8::fast_api::CTypeInfo;
21+
use v8::fast_api::CFunction;
2422
use v8::Isolate;
2523

2624
pub type PromiseId = i32;
@@ -74,12 +72,6 @@ impl OpMetadata {
7472
}
7573
}
7674

77-
#[derive(Clone, Copy)]
78-
pub(crate) struct FastFunctionInfo {
79-
pub(crate) fn_info: NonNull<CFunctionInfo>,
80-
pub(crate) fn_sig: (NonNull<CTypeInfo>, NonNull<CTypeInfo>),
81-
}
82-
8375
/// Per-op context.
8476
///
8577
// Note: We don't worry too much about the size of this struct because it's allocated once per realm, and is
@@ -98,7 +90,7 @@ pub struct OpCtx {
9890
pub get_error_class_fn: GetErrorClassFn,
9991

10092
pub(crate) decl: OpDecl,
101-
pub(crate) fast_fn_info: Option<FastFunctionInfo>,
93+
pub(crate) fast_fn_info: Option<CFunction>,
10294
pub(crate) metrics_fn: Option<OpMetricsFn>,
10395
/// If the last fast op failed, stores the error to be picked up by the slow op.
10496
pub(crate) last_fast_error: UnsafeCell<Option<AnyError>>,
@@ -120,35 +112,15 @@ impl OpCtx {
120112
metrics_fn: Option<OpMetricsFn>,
121113
) -> Self {
122114
// If we want metrics for this function, create the fastcall `CFunctionInfo` from the metrics
123-
// `FastFunction`. For some extremely fast ops, the parameter list may change for the metrics
115+
// `CFunction`. For some extremely fast ops, the parameter list may change for the metrics
124116
// version and require a slightly different set of arguments (for example, it may need the fastcall
125117
// callback information to get the `OpCtx`).
126-
let fast_fn = if metrics_fn.is_some() {
127-
&decl.fast_fn_with_metrics
118+
let fast_fn_info = if metrics_fn.is_some() {
119+
decl.fast_fn_with_metrics
128120
} else {
129-
&decl.fast_fn
121+
decl.fast_fn
130122
};
131123

132-
let fast_fn_info = fast_fn.map(|fast_fn| {
133-
let args = CTypeInfo::new_from_slice(fast_fn.args);
134-
let ret = CTypeInfo::new(fast_fn.return_type);
135-
136-
// SAFETY: all arguments are coming from the trait and they have
137-
// static lifetime
138-
let c_fn = unsafe {
139-
CFunctionInfo::new(
140-
args.as_ptr(),
141-
fast_fn.args.len(),
142-
ret.as_ptr(),
143-
fast_fn.repr,
144-
)
145-
};
146-
FastFunctionInfo {
147-
fn_info: c_fn,
148-
fn_sig: (args, ret),
149-
}
150-
});
151-
152124
Self {
153125
id,
154126
state,
@@ -190,13 +162,13 @@ impl OpCtx {
190162
function: self.decl.slow_fn_with_metrics,
191163
};
192164
if let (Some(fast_fn), Some(fast_fn_info)) =
193-
(&self.decl.fast_fn_with_metrics, &self.fast_fn_info)
165+
(self.decl.fast_fn_with_metrics, self.fast_fn_info)
194166
{
195167
let fast_fn = v8::ExternalReference {
196-
pointer: fast_fn.function as _,
168+
pointer: fast_fn.address() as _,
197169
};
198170
let fast_info = v8::ExternalReference {
199-
pointer: fast_fn_info.fn_info.as_ptr() as _,
171+
type_info: fast_fn_info.type_info(),
200172
};
201173
[ctx_ptr, slow_fn, fast_fn, fast_info]
202174
} else {
@@ -207,13 +179,13 @@ impl OpCtx {
207179
function: self.decl.slow_fn,
208180
};
209181
if let (Some(fast_fn), Some(fast_fn_info)) =
210-
(&self.decl.fast_fn, &self.fast_fn_info)
182+
(self.decl.fast_fn, self.fast_fn_info)
211183
{
212184
let fast_fn = v8::ExternalReference {
213-
pointer: fast_fn.function as _,
185+
pointer: fast_fn.address() as _,
214186
};
215187
let fast_info = v8::ExternalReference {
216-
pointer: fast_fn_info.fn_info.as_ptr() as _,
188+
type_info: fast_fn_info.type_info(),
217189
};
218190
[ctx_ptr, slow_fn, fast_fn, fast_info]
219191
} else {

core/runtime/bindings.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -392,14 +392,8 @@ fn op_ctx_function<'s>(
392392
})
393393
.length(op_ctx.decl.arg_count as i32);
394394

395-
let template = if let Some(fast_function) = &fast_fn {
396-
builder.build_fast(
397-
scope,
398-
fast_function,
399-
Some(op_ctx.fast_fn_info.unwrap().fn_info.as_ptr()),
400-
None,
401-
None,
402-
)
395+
let template = if let Some(fast_function) = fast_fn {
396+
builder.build_fast(scope, &[fast_function])
403397
} else {
404398
builder.build(scope)
405399
};

core/runtime/jsruntime.rs

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ use crate::modules::ModuleMap;
3939
use crate::modules::ModuleName;
4040
use crate::modules::RequestedModuleType;
4141
use crate::modules::ValidateImportAttributesCb;
42-
use crate::ops::FastFunctionInfo;
4342
use crate::ops_metrics::dispatch_metrics_async;
4443
use crate::ops_metrics::OpMetricsFactoryFn;
4544
use crate::runtime::ContextState;
@@ -152,7 +151,6 @@ pub(crate) struct InnerIsolateState {
152151
main_realm: ManuallyDrop<JsRealm>,
153152
pub(crate) state: ManuallyDropRc<JsRuntimeState>,
154153
v8_isolate: ManuallyDrop<v8::OwnedIsolate>,
155-
fast_fn_infos: Vec<FastFunctionInfo>,
156154
}
157155

158156
impl InnerIsolateState {
@@ -220,20 +218,6 @@ impl Drop for InnerIsolateState {
220218
ManuallyDrop::drop(&mut self.v8_isolate);
221219
}
222220
}
223-
// Free the fast function infos manually.
224-
for FastFunctionInfo {
225-
fn_info,
226-
fn_sig: (args, ret),
227-
} in std::mem::take(&mut self.fast_fn_infos)
228-
{
229-
// SAFETY: We logically own these, and there are no remaining references because we just destroyed the
230-
// realm and isolate above.
231-
unsafe {
232-
std::ptr::drop_in_place(fn_info.as_ptr());
233-
std::ptr::drop_in_place(args.as_ptr());
234-
std::ptr::drop_in_place(ret.as_ptr());
235-
}
236-
}
237221
}
238222
}
239223

@@ -942,13 +926,6 @@ impl JsRuntime {
942926
}
943927
op_state.borrow_mut().put(isolate_ptr);
944928

945-
let mut fast_fn_infos = Vec::with_capacity(op_ctxs.len());
946-
for op_ctx in &*op_ctxs {
947-
if let Some(fast_fn_info) = op_ctx.fast_fn_info {
948-
fast_fn_infos.push(fast_fn_info);
949-
}
950-
}
951-
952929
// ...once ops and isolate are set up, we can create a `ContextState`...
953930
let context_state = Rc::new(ContextState::new(
954931
op_driver.clone(),
@@ -1107,7 +1084,6 @@ impl JsRuntime {
11071084
main_realm: ManuallyDrop::new(main_realm),
11081085
state: ManuallyDropRc(ManuallyDrop::new(state_rc)),
11091086
v8_isolate: ManuallyDrop::new(isolate),
1110-
fast_fn_infos,
11111087
},
11121088
allocations: isolate_allocations,
11131089
files_loaded_from_fs_during_snapshot: vec![],

dcore/src/inspector_server.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ async fn server(
271271
let json_version_response = json!({
272272
"Browser": name,
273273
"Protocol-Version": "1.3",
274-
"V8-Version": deno_core::v8_version(),
274+
"V8-Version": deno_core::v8::VERSION_STRING,
275275
});
276276

277277
// Create the server manually so it can use the Local Executor

0 commit comments

Comments
 (0)