Skip to content

Commit addaff2

Browse files
Sujay JayakarConvex, Inc.
Sujay Jayakar
authored and
Convex, Inc.
committed
Add OpProvider for sharing op implementations between isolate1 and isolate2 (#24639)
Pretty straightforward application of the provider trait approach for syscalls to ops. GitOrigin-RevId: ee0520c63e52936f946f897e4cd1dfbf7b9e8fff
1 parent 76f55b7 commit addaff2

25 files changed

+1139
-998
lines changed

‎crates/convex_macro/src/lib.rs

Lines changed: 16 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use syn::{
55
FnArg,
66
GenericArgument,
77
ItemFn,
8+
Pat,
89
PathArguments,
910
PathSegment,
1011
ReturnType,
@@ -159,7 +160,7 @@ pub fn instrument_future(_attr: TokenStream, item: TokenStream) -> TokenStream {
159160
/// undefined), while both `null` and `undefined` (and missing positional)
160161
/// arguments become None.
161162
///
162-
/// The function should be called as `scope.op_name(args, rt)?`.
163+
/// The function should be called as `op_name(provider, args, rt)?`.
163164
#[proc_macro_attribute]
164165
pub fn v8_op(_attr: TokenStream, item: TokenStream) -> TokenStream {
165166
let ItemFn {
@@ -186,106 +187,14 @@ pub fn v8_op(_attr: TokenStream, item: TokenStream) -> TokenStream {
186187
..
187188
} = sig;
188189

189-
let Some(FnArg::Receiver(receiver)) = inputs.first() else {
190-
panic!("op should take &mut self");
190+
let Some(FnArg::Typed(first_pat_type)) = inputs.first() else {
191+
panic!("op should take a first argument for its op provider");
191192
};
192-
let arg_parsing: TokenStream2 = inputs
193-
.iter()
194-
.enumerate()
195-
.skip(1)
196-
.map(|(idx, input)| {
197-
let idx = idx as i32;
198-
let FnArg::Typed(pat) = input else {
199-
panic!("input must be typed")
200-
};
201-
let arg_info = format!("{} arg{}", ident, idx);
202-
// NOTE: deno has special case when pat.ty is &mut [u8].
203-
// While that would make some ops more efficient, it also makes them
204-
// unsafe because it's hard to prove that the same buffer isn't
205-
// being mutated from multiple ops in parallel or multiple arguments
206-
// on the same op.
207-
//
208-
// Forego all special casing and just use serde_v8.
209-
quote! {
210-
let #pat = {
211-
let __raw_arg = __args.get(#idx);
212-
::deno_core::serde_v8::from_v8(self, __raw_arg).context(#arg_info)?
213-
};
214-
}
215-
})
216-
.collect();
217-
218-
let ReturnType::Type(_, return_type) = output else {
219-
panic!("op needs return type");
220-
};
221-
let Type::Path(rtype_path) = &**return_type else {
222-
panic!("op must return anyhow::Result<...>")
223-
};
224-
let PathSegment {
225-
ident: retval_type,
226-
arguments: retval_arguments,
227-
} = rtype_path.path.segments.last().unwrap();
228-
assert_eq!(&retval_type.to_string(), "Result");
229-
let PathArguments::AngleBracketed(retval_arguments) = retval_arguments else {
230-
panic!("op must return anyhow::Result<...>")
231-
};
232-
let GenericArgument::Type(_retval_type) = retval_arguments
233-
.args
234-
.last()
235-
.expect("op must return anyhow::Result<...>")
236-
else {
237-
panic!("op must return anyhow::Result<...>");
193+
let Pat::Ident(first_pat_ident) = &*first_pat_type.pat else {
194+
panic!("op's first argument should be a plain identifier");
238195
};
239-
let serialize_retval = quote! {
240-
let __value_v8 = deno_core::serde_v8::to_v8(self, __result_v)?;
241-
__rv.set(__value_v8);
242-
};
243-
244-
let gen = quote! {
245-
#(#attrs)*
246-
#vis fn #ident #generics (
247-
#receiver,
248-
__args: ::deno_core::v8::FunctionCallbackArguments,
249-
mut __rv: ::deno_core::v8::ReturnValue,
250-
) -> ::anyhow::Result<()> {
251-
#arg_parsing
252-
let __result_v = (|| #output { #block })()?;
253-
{ #serialize_retval }
254-
Ok(())
255-
}
256-
};
257-
gen.into()
258-
}
259-
260-
#[proc_macro_attribute]
261-
pub fn v8_op2(_attr: TokenStream, item: TokenStream) -> TokenStream {
262-
let ItemFn {
263-
ref attrs,
264-
ref vis,
265-
ref sig,
266-
ref block,
267-
} = syn::parse(item).unwrap();
268-
269-
assert!(sig.constness.is_none(), "const fn cannot be op");
270-
assert!(sig.asyncness.is_none(), "async fn cannot be op");
271-
assert!(sig.unsafety.is_none(), "unsafe fn cannot be op");
272-
assert!(sig.abi.is_none(), "fn with explicit ABI cannot be op");
273-
assert!(
274-
sig.variadic.is_none(),
275-
"fn with variadic arguments cannot be op"
276-
);
196+
let provider_ident = &first_pat_ident.ident;
277197

278-
let Signature {
279-
ref ident,
280-
ref generics,
281-
ref inputs,
282-
ref output,
283-
..
284-
} = sig;
285-
286-
let Some(FnArg::Receiver(receiver)) = inputs.first() else {
287-
panic!("op should take &mut self");
288-
};
289198
let arg_parsing: TokenStream2 = inputs
290199
.iter()
291200
.enumerate()
@@ -306,7 +215,10 @@ pub fn v8_op2(_attr: TokenStream, item: TokenStream) -> TokenStream {
306215
quote! {
307216
let #pat = {
308217
let __raw_arg = __args.get(#idx);
309-
::deno_core::serde_v8::from_v8(self.scope, __raw_arg).context(#arg_info)?
218+
::deno_core::serde_v8::from_v8(
219+
OpProvider::scope(#provider_ident),
220+
__raw_arg,
221+
).context(#arg_info)?
310222
};
311223
}
312224
})
@@ -334,14 +246,17 @@ pub fn v8_op2(_attr: TokenStream, item: TokenStream) -> TokenStream {
334246
panic!("op must return anyhow::Result<...>");
335247
};
336248
let serialize_retval = quote! {
337-
let __value_v8 = deno_core::serde_v8::to_v8(self.scope, __result_v)?;
249+
let __value_v8 = deno_core::serde_v8::to_v8(
250+
OpProvider::scope(#provider_ident),
251+
__result_v,
252+
)?;
338253
__rv.set(__value_v8);
339254
};
340255

341256
let gen = quote! {
342257
#(#attrs)*
343258
#vis fn #ident #generics (
344-
#receiver,
259+
#first_pat_type,
345260
__args: ::deno_core::v8::FunctionCallbackArguments,
346261
mut __rv: ::deno_core::v8::ReturnValue,
347262
) -> ::anyhow::Result<()> {

‎crates/isolate/src/environment/action/mod.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,8 +1024,6 @@ impl<RT: Runtime> ActionEnvironment<RT> {
10241024
}
10251025

10261026
impl<RT: Runtime> IsolateEnvironment<RT> for ActionEnvironment<RT> {
1027-
type Rng = ChaCha12Rng;
1028-
10291027
fn trace(&mut self, level: LogLevel, messages: Vec<String>) -> anyhow::Result<()> {
10301028
// - 1 to reserve for the [ERROR] log line
10311029

@@ -1072,7 +1070,7 @@ impl<RT: Runtime> IsolateEnvironment<RT> for ActionEnvironment<RT> {
10721070
Ok(())
10731071
}
10741072

1075-
fn rng(&mut self) -> anyhow::Result<&mut Self::Rng> {
1073+
fn rng(&mut self) -> anyhow::Result<&mut ChaCha12Rng> {
10761074
self.phase.rng()
10771075
}
10781076

‎crates/isolate/src/environment/analyze.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,6 @@ pub struct AnalyzeEnvironment {
123123
}
124124

125125
impl<RT: Runtime> IsolateEnvironment<RT> for AnalyzeEnvironment {
126-
type Rng = ChaCha12Rng;
127-
128126
fn trace(&mut self, _level: LogLevel, messages: Vec<String>) -> anyhow::Result<()> {
129127
tracing::warn!(
130128
"Unexpected Console access at import time: {}",
@@ -146,7 +144,7 @@ impl<RT: Runtime> IsolateEnvironment<RT> for AnalyzeEnvironment {
146144
Ok(())
147145
}
148146

149-
fn rng(&mut self) -> anyhow::Result<&mut Self::Rng> {
147+
fn rng(&mut self) -> anyhow::Result<&mut ChaCha12Rng> {
150148
Ok(&mut self.rng)
151149
}
152150

‎crates/isolate/src/environment/auth_config.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,6 @@ pub struct AuthConfig {
7575
}
7676

7777
impl<RT: Runtime> IsolateEnvironment<RT> for AuthConfigEnvironment {
78-
type Rng = ChaCha12Rng;
79-
8078
fn trace(&mut self, _level: LogLevel, messages: Vec<String>) -> anyhow::Result<()> {
8179
tracing::warn!(
8280
"Unexpected Console access when evaluating auth config file: {}",
@@ -98,7 +96,7 @@ impl<RT: Runtime> IsolateEnvironment<RT> for AuthConfigEnvironment {
9896
Ok(())
9997
}
10098

101-
fn rng(&mut self) -> anyhow::Result<&mut Self::Rng> {
99+
fn rng(&mut self) -> anyhow::Result<&mut ChaCha12Rng> {
102100
anyhow::bail!(ErrorMetadata::bad_request(
103101
"NoRandomDuringAuthConfig",
104102
"Math.random unsupported when evaluating auth config file"

‎crates/isolate/src/environment/mod.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use model::modules::module_versions::{
2929
ModuleSource,
3030
SourceMap,
3131
};
32-
use rand::Rng;
32+
use rand_chacha::ChaCha12Rng;
3333
use serde_json::Value as JsonValue;
3434
use value::{
3535
TableMapping,
@@ -62,8 +62,6 @@ use crate::{
6262
/// Both ops and syscalls can return errors tagged with `DeveloperError` to
6363
/// signal a user-visible error that will be turned into a JavaScript exception.
6464
pub trait IsolateEnvironment<RT: Runtime>: 'static {
65-
type Rng: Rng;
66-
6765
#[allow(async_fn_in_trait)]
6866
async fn lookup_source(
6967
&mut self,
@@ -87,7 +85,7 @@ pub trait IsolateEnvironment<RT: Runtime>: 'static {
8785
messages: Vec<String>,
8886
system_log_metadata: SystemLogMetadata,
8987
) -> anyhow::Result<()>;
90-
fn rng(&mut self) -> anyhow::Result<&mut Self::Rng>;
88+
fn rng(&mut self) -> anyhow::Result<&mut ChaCha12Rng>;
9189
fn unix_timestamp(&self) -> anyhow::Result<UnixTimestamp>;
9290

9391
fn get_environment_variable(&mut self, name: EnvVarName)

‎crates/isolate/src/environment/schema.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ pub struct SchemaEnvironment {
7272
}
7373

7474
impl<RT: Runtime> IsolateEnvironment<RT> for SchemaEnvironment {
75-
type Rng = ChaCha12Rng;
76-
7775
fn trace(&mut self, _level: LogLevel, messages: Vec<String>) -> anyhow::Result<()> {
7876
tracing::warn!(
7977
"Unexpected Console access at schema evaluation time: {}",
@@ -95,7 +93,7 @@ impl<RT: Runtime> IsolateEnvironment<RT> for SchemaEnvironment {
9593
Ok(())
9694
}
9795

98-
fn rng(&mut self) -> anyhow::Result<&mut Self::Rng> {
96+
fn rng(&mut self) -> anyhow::Result<&mut ChaCha12Rng> {
9997
Ok(&mut self.rng)
10098
}
10199

‎crates/isolate/src/environment/udf/mod.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,6 @@ pub struct DatabaseUdfEnvironment<RT: Runtime> {
203203
}
204204

205205
impl<RT: Runtime> IsolateEnvironment<RT> for DatabaseUdfEnvironment<RT> {
206-
type Rng = ChaCha12Rng;
207-
208206
fn trace(&mut self, level: LogLevel, messages: Vec<String>) -> anyhow::Result<()> {
209207
// - 1 to reserve for the [ERROR] log line
210208
match self.log_lines.len().cmp(&(MAX_LOG_LINES - 1)) {
@@ -249,7 +247,7 @@ impl<RT: Runtime> IsolateEnvironment<RT> for DatabaseUdfEnvironment<RT> {
249247
Ok(())
250248
}
251249

252-
fn rng(&mut self) -> anyhow::Result<&mut Self::Rng> {
250+
fn rng(&mut self) -> anyhow::Result<&mut ChaCha12Rng> {
253251
self.phase.rng()
254252
}
255253

‎crates/isolate/src/execution_scope.rs

Lines changed: 0 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -524,85 +524,6 @@ impl<'a, 'b: 'a, RT: Runtime, E: IsolateEnvironment<RT>> ExecutionScope<'a, 'b,
524524
Ok(v8::Local::new(&mut scope, handle))
525525
}
526526

527-
pub fn op(
528-
&mut self,
529-
args: v8::FunctionCallbackArguments,
530-
rv: v8::ReturnValue,
531-
) -> anyhow::Result<()> {
532-
let _s = static_span!();
533-
if args.length() < 1 {
534-
// This must be a bug in our `udf-runtime` code, not a developer error.
535-
anyhow::bail!("op(op_name, ...) takes at least one argument");
536-
}
537-
let op_name: v8::Local<v8::String> = args.get(0).try_into()?;
538-
let op_name = to_rust_string(self, &op_name)?;
539-
540-
let timer = metrics::op_timer(&op_name);
541-
542-
match &op_name[..] {
543-
"throwUncatchableDeveloperError" => self.op_throwUncatchableDeveloperError(args, rv)?,
544-
"console/message" => self.op_console_message(args, rv)?,
545-
"console/trace" => self.op_console_trace(args, rv)?,
546-
"console/timeStart" => self.op_console_timeStart(args, rv)?,
547-
"console/timeLog" => self.op_console_timeLog(args, rv)?,
548-
"console/timeEnd" => self.op_console_timeEnd(args, rv)?,
549-
"error/stack" => self.op_error_stack(args, rv)?,
550-
"random" => self.op_random(args, rv)?,
551-
"now" => self.op_now(args, rv)?,
552-
"url/getUrlInfo" => self.op_url_getUrlInfo(args, rv)?,
553-
"url/getUrlSearchParamPairs" => self.op_url_getUrlSearchParamPairs(args, rv)?,
554-
"url/stringifyUrlSearchParams" => self.op_url_stringifyUrlSearchParams(args, rv)?,
555-
"url/updateUrlInfo" => self.op_url_updateUrlInfo(args, rv)?,
556-
"blob/createPart" => self.op_blob_createPart(args, rv)?,
557-
"blob/slicePart" => self.op_blob_slicePart(args, rv)?,
558-
"blob/readPart" => self.op_blob_readPart(args, rv)?,
559-
"stream/create" => self.op_stream_create(args, rv)?,
560-
"stream/extend" => self.op_stream_extend(args, rv)?,
561-
"headers/getMimeType" => self.op_headers_getMimeType(args, rv)?,
562-
"headers/normalizeName" => self.op_headers_normalizeName(args, rv)?,
563-
"textEncoder/encode" => self.op_textEncoder_encode(args, rv)?,
564-
"textEncoder/encodeInto" => self.op_textEncoder_encodeInto(args, rv)?,
565-
"textEncoder/decode" => self.op_textEncoder_decode(args, rv)?,
566-
"textEncoder/normalizeLabel" => self.op_textEncoder_normalizeLabel(args, rv)?,
567-
"atob" => self.op_atob(args, rv)?,
568-
"btoa" => self.op_btoa(args, rv)?,
569-
"environmentVariables/get" => self.op_environmentVariables_get(args, rv)?,
570-
"getTableMappingWithoutSystemTables" => {
571-
self.op_getTableMappingWithoutSystemTables(args, rv)?
572-
},
573-
"validateArgs" => self.op_validate_args(args, rv)?,
574-
"crypto/randomUUID" => self.op_crypto_randomUUID(args, rv)?,
575-
"crypto/getRandomValues" => self.op_crypto_getRandomValues(args, rv)?,
576-
"crypto/sign" => self.op_crypto_sign(args, rv)?,
577-
"crypto/signEd25519" => self.op_crypto_sign_ed25519(args, rv)?,
578-
"crypto/verify" => self.op_crypto_verify(args, rv)?,
579-
"crypto/verifyEd25519" => self.op_crypto_verify_ed25519(args, rv)?,
580-
"crypto/deriveBits" => self.op_crypto_deriveBits(args, rv)?,
581-
"crypto/digest" => self.op_crypto_digest(args, rv)?,
582-
"crypto/importKey" => self.op_crypto_importKey(args, rv)?,
583-
"crypto/importSpkiEd25519" => self.op_crypto_import_spki_ed25519(args, rv)?,
584-
"crypto/importPkcs8Ed25519" => self.op_crypto_import_pkcs8_ed25519(args, rv)?,
585-
"crypto/importSpkiX25519" => self.op_crypto_import_spki_x25519(args, rv)?,
586-
"crypto/importPkcs8X25519" => self.op_crypto_import_pkcs8_x25519(args, rv)?,
587-
"crypto/base64UrlEncode" => self.op_crypto_base64_url_encode(args, rv)?,
588-
"crypto/base64UrlDecode" => self.op_crypto_base64_url_decode(args, rv)?,
589-
"crypto/exportKey" => self.op_crypto_export_key(args, rv)?,
590-
"crypto/exportSpkiEd25519" => self.op_crypto_export_spki_ed25519(args, rv)?,
591-
"crypto/exportPkcs8Ed25519" => self.op_crypto_export_pkcs8_ed25519(args, rv)?,
592-
"crypto/JwkXEd25519" => self.op_crypto_jwk_x_ed25519(args, rv)?,
593-
"crypto/exportSpkiX25519" => self.op_crypto_export_spki_x25519(args, rv)?,
594-
"crypto/exportPkcs8X25519" => self.op_crypto_export_pkcs8_x25519(args, rv)?,
595-
_ => {
596-
anyhow::bail!(ErrorMetadata::bad_request(
597-
"UnknownOperation",
598-
format!("Unknown operation {op_name}")
599-
));
600-
},
601-
};
602-
timer.finish();
603-
Ok(())
604-
}
605-
606527
pub fn async_op(
607528
&mut self,
608529
args: v8::FunctionCallbackArguments,

‎crates/isolate/src/isolate2/context.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ use super::{
2222
session::Session,
2323
FunctionId,
2424
};
25-
use crate::strings;
25+
use crate::{
26+
ops::run_op,
27+
strings,
28+
};
2629

2730
// Each isolate session can have multiple contexts, which we'll eventually use
2831
// for subtransactions. Each context executes with a particular environment,
@@ -183,15 +186,12 @@ impl Context {
183186
rv: v8::ReturnValue,
184187
) {
185188
let mut ctx = EnteredContext::from_callback(scope);
186-
match ctx.op(args, rv) {
187-
Ok(()) => (),
188-
Err(e) => {
189-
// XXX: Handle syscall or op error.
190-
// let message = strings::syscallError.create(scope).unwrap();
191-
// let exception = v8::Exception::error(scope, message);
192-
// scope.throw_exception(exception);
193-
panic!("Unexpected error: {e:?}");
194-
},
189+
if let Err(e) = run_op(&mut ctx, args, rv) {
190+
// XXX: Handle syscall or op error.
191+
// let message = strings::syscallError.create(scope).unwrap();
192+
// let exception = v8::Exception::error(scope, message);
193+
// scope.throw_exception(exception);
194+
panic!("Unexpected error: {e:?}");
195195
}
196196
}
197197

0 commit comments

Comments
 (0)