Skip to content

Commit 001c084

Browse files
committed
Speed named call dispatch in eval
1 parent 1eebc32 commit 001c084

4 files changed

Lines changed: 82 additions & 23 deletions

File tree

neovm-core/src/emacs_core/builtins/symbols.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ pub(crate) fn set_default_toplevel_value_impl(
358358
value,
359359
) {
360360
ctx.obarray.set_symbol_value_id(resolved, value);
361+
ctx.sync_cached_runtime_binding_by_id(resolved, value);
361362
}
362363
ctx.refresh_gc_runtime_settings_after_change_by_id(resolved);
363364
Ok(Value::NIL)

neovm-core/src/emacs_core/bytecode/vm.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1756,8 +1756,7 @@ impl<'a> Vm<'a> {
17561756
}
17571757

17581758
fn named_builtin_fast_path_allowed_id(&self, id: SymId) -> bool {
1759-
if crate::emacs_core::eval::compiler_function_overrides_active_in_obarray(&self.ctx.obarray)
1760-
{
1759+
if self.ctx.compiler_function_overrides_active() {
17611760
return false;
17621761
}
17631762
match self.ctx.obarray.symbol_function_id(id) {

neovm-core/src/emacs_core/eval.rs

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -243,13 +243,9 @@ pub(crate) fn clear_global_subr_table() {
243243

244244
/// Cached SymId for `internal--compiler-function-overrides`.
245245
///
246-
/// `compiler_function_overrides_active_in_obarray` is called from
247-
/// `resolve_named_call_target_by_id` on every funcall. The previous
248-
/// implementation re-interned the string each call, which acquires the
249-
/// global interner write lock — even after `parking_lot::RwLock`, that
250-
/// is many extra atomic ops per call and shows up as the dominant cost
251-
/// in debug-build batch-byte-compile profiles. Cache the SymId once
252-
/// and use the `_id`-suffixed obarray accessor that bypasses intern.
246+
/// Hot evaluator and bytecode dispatch paths cache whether this variable has a
247+
/// cons value. Keep the SymId cached as well so the mutation paths can refresh
248+
/// that flag without re-interning the string.
253249
fn internal_compiler_function_overrides_sym() -> SymId {
254250
static SYM: OnceLock<SymId> = OnceLock::new();
255251
*SYM.get_or_init(|| intern(INTERNAL_COMPILER_FUNCTION_OVERRIDES))
@@ -301,11 +297,6 @@ pub(crate) fn compiler_function_override_in_obarray(
301297
None
302298
}
303299

304-
pub(crate) fn compiler_function_overrides_active_in_obarray(obarray: &Obarray) -> bool {
305-
let overrides_sym = internal_compiler_function_overrides_sym();
306-
obarray.symbol_value_id_or_nil(overrides_sym).is_cons()
307-
}
308-
309300
#[derive(Clone, Debug)]
310301
struct ExecutingKbdMacroRuntimeScope {
311302
snapshot: crate::keyboard::ExecutingKbdMacroRuntimeSnapshot,
@@ -1040,6 +1031,7 @@ fn cons_head_symbol_id(value: &Value) -> Option<SymId> {
10401031

10411032
struct CoreEvalSymbols {
10421033
internal_interpreter_environment_symbol: SymId,
1034+
compiler_function_overrides_symbol: SymId,
10431035
quit_flag_symbol: SymId,
10441036
inhibit_quit_symbol: SymId,
10451037
throw_on_input_symbol: SymId,
@@ -1055,6 +1047,8 @@ fn install_core_eval_symbols(obarray: &mut Obarray, reset_runtime_values: bool)
10551047
obarray.set_symbol_value_id(internal_interpreter_environment_symbol, Value::NIL);
10561048
obarray.make_special_id(internal_interpreter_environment_symbol);
10571049

1050+
let compiler_function_overrides_symbol = internal_compiler_function_overrides_sym();
1051+
10581052
let quit_flag_symbol = intern("quit-flag");
10591053
if reset_runtime_values {
10601054
obarray.set_symbol_value_id(quit_flag_symbol, Value::NIL);
@@ -1080,6 +1074,7 @@ fn install_core_eval_symbols(obarray: &mut Obarray, reset_runtime_values: bool)
10801074

10811075
CoreEvalSymbols {
10821076
internal_interpreter_environment_symbol,
1077+
compiler_function_overrides_symbol,
10831078
quit_flag_symbol,
10841079
inhibit_quit_symbol,
10851080
throw_on_input_symbol,
@@ -1690,6 +1685,14 @@ pub struct Context {
16901685
next_resume_id: u64,
16911686
/// GNU `pending_funcalls` equivalent for internal no-Lisp teardown paths.
16921687
pub(crate) pending_safe_funcalls: Vec<PendingSafeFuncall>,
1688+
/// Cached truth of `internal--compiler-function-overrides`.
1689+
///
1690+
/// GNU's hot evaluator path reads the function cell directly. Neomacs only
1691+
/// needs the override alist during compiler/macro machinery, so keep the
1692+
/// nil/common case as a cached flag and refresh it through the same runtime
1693+
/// binding paths that already maintain `quit-flag` and `noninteractive`.
1694+
compiler_function_overrides_symbol: SymId,
1695+
compiler_function_overrides_active: bool,
16931696
/// Hot cache for named callable resolution in `funcall`/`apply`.
16941697
/// Keyed by symbol id; entries are validated against the obarray's
16951698
/// `function_epoch` so that any `defalias` / `fset` / autoload
@@ -4249,6 +4252,9 @@ impl Context {
42494252
let print_symbols_bare = obarray
42504253
.symbol_value_id_or_nil(core_eval_symbols.print_symbols_bare_symbol)
42514254
.is_truthy();
4255+
let compiler_function_overrides_active = obarray
4256+
.symbol_value_id_or_nil(core_eval_symbols.compiler_function_overrides_symbol)
4257+
.is_cons();
42524258
let quit_flag = obarray.symbol_value_id_or_nil(core_eval_symbols.quit_flag_symbol);
42534259
let inhibit_quit = obarray.symbol_value_id_or_nil(core_eval_symbols.inhibit_quit_symbol);
42544260

@@ -4334,6 +4340,9 @@ impl Context {
43344340
condition_stack: Vec::new(),
43354341
next_resume_id: 1,
43364342
pending_safe_funcalls: Vec::new(),
4343+
compiler_function_overrides_symbol: core_eval_symbols
4344+
.compiler_function_overrides_symbol,
4345+
compiler_function_overrides_active,
43374346
named_call_cache: HashMap::with_capacity(NAMED_CALL_CACHE_CAPACITY),
43384347
lexenv_assq_cache: LexenvAssqCache::default(),
43394348
lexenv_special_cache: LexenvSpecialCache::default(),
@@ -4402,6 +4411,9 @@ impl Context {
44024411
let print_symbols_bare = obarray
44034412
.symbol_value_id_or_nil(core_eval_symbols.print_symbols_bare_symbol)
44044413
.is_truthy();
4414+
let compiler_function_overrides_active = obarray
4415+
.symbol_value_id_or_nil(core_eval_symbols.compiler_function_overrides_symbol)
4416+
.is_cons();
44054417
let quit_flag = obarray.symbol_value_id_or_nil(core_eval_symbols.quit_flag_symbol);
44064418
let inhibit_quit = obarray.symbol_value_id_or_nil(core_eval_symbols.inhibit_quit_symbol);
44074419

@@ -4487,6 +4499,9 @@ impl Context {
44874499
condition_stack: Vec::new(),
44884500
next_resume_id: 1,
44894501
pending_safe_funcalls: Vec::new(),
4502+
compiler_function_overrides_symbol: core_eval_symbols
4503+
.compiler_function_overrides_symbol,
4504+
compiler_function_overrides_active,
44904505
named_call_cache: HashMap::with_capacity(NAMED_CALL_CACHE_CAPACITY),
44914506
lexenv_assq_cache: LexenvAssqCache::default(),
44924507
lexenv_special_cache: LexenvSpecialCache::default(),
@@ -6428,11 +6443,13 @@ impl Context {
64286443
}
64296444

64306445
#[inline]
6431-
fn sync_cached_runtime_binding_by_id(&mut self, sym_id: SymId, value: Value) {
6446+
pub(crate) fn sync_cached_runtime_binding_by_id(&mut self, sym_id: SymId, value: Value) {
64326447
if sym_id == self.quit_flag_symbol {
64336448
self.quit_flag = value;
64346449
} else if sym_id == self.inhibit_quit_symbol {
64356450
self.inhibit_quit = value;
6451+
} else if sym_id == self.compiler_function_overrides_symbol {
6452+
self.compiler_function_overrides_active = value.is_cons();
64366453
} else if sym_id == self.noninteractive_symbol {
64376454
self.noninteractive = value.is_truthy();
64386455
} else if sym_id == self.symbols_with_pos_enabled_symbol {
@@ -6442,6 +6459,11 @@ impl Context {
64426459
}
64436460
}
64446461

6462+
#[inline(always)]
6463+
pub(crate) fn compiler_function_overrides_active(&self) -> bool {
6464+
self.compiler_function_overrides_active
6465+
}
6466+
64456467
fn sync_keyboard_runtime_binding_by_id(&mut self, sym_id: SymId, value: Value) {
64466468
if sym_id == intern("input-decode-map") {
64476469
self.command_loop.keyboard.set_input_decode_map(value);
@@ -7020,8 +7042,10 @@ impl Context {
70207042

70217043
// Resolve function value
70227044
let func = if let Some(sym_id) = sym_id {
7023-
if let Some(override_func) =
7024-
compiler_function_override_in_obarray(&self.obarray, sym_id)
7045+
if let Some(override_func) = self
7046+
.compiler_function_overrides_active()
7047+
.then(|| compiler_function_override_in_obarray(&self.obarray, sym_id))
7048+
.flatten()
70257049
{
70267050
override_func
70277051
} else {
@@ -7222,8 +7246,6 @@ impl Context {
72227246
self.set_backtrace_args_evalled_owned(outer_bt_count, args);
72237247
self.unbind_to(args_roots_base);
72247248

7225-
self.maybe_gc_and_quit()?;
7226-
72277249
return self.maybe_grow_eval_stack(|ctx| {
72287250
ctx.dispatch_subr_entry_from_backtrace_unchecked(entry, outer_bt_count)
72297251
.unwrap_or_else(|| {
@@ -7236,8 +7258,6 @@ impl Context {
72367258
self.set_backtrace_args_evalled(outer_bt_count, &args);
72377259
self.unbind_to(args_roots_base);
72387260

7239-
self.maybe_gc_and_quit()?;
7240-
72417261
if let Some((sym_id, entry)) = direct_subr_entry {
72427262
return self.maybe_grow_eval_stack(|ctx| {
72437263
if entry.dispatch_kind == SubrDispatchKind::ContextCallable {
@@ -10442,8 +10462,7 @@ impl Context {
1044210462

1044310463
#[inline]
1044410464
fn resolve_named_call_target_by_id(&mut self, sym_id: SymId) -> NamedCallTarget {
10445-
let compiler_overrides_active =
10446-
compiler_function_overrides_active_in_obarray(&self.obarray);
10465+
let compiler_overrides_active = self.compiler_function_overrides_active();
1044710466
let function_epoch = self.obarray.function_epoch();
1044810467
if !compiler_overrides_active {
1044910468
// Fast path: a HashMap lookup that returns the cached target

neovm-core/src/emacs_core/eval_test.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6144,6 +6144,46 @@ fn named_call_cache_invalidates_on_function_cell_mutation() {
61446144
assert_eq!(results[4], "OK 11");
61456145
}
61466146

6147+
#[test]
6148+
fn compiler_function_overrides_cache_tracks_dynamic_binding() {
6149+
crate::test_utils::init_test_tracing();
6150+
assert_eq!(
6151+
eval_one(
6152+
r#"(progn
6153+
(fset 'neomacs--override-target (lambda () 'base))
6154+
(list (neomacs--override-target)
6155+
(let ((internal--compiler-function-overrides
6156+
(list (cons 'neomacs--override-target
6157+
(lambda () 'override)))))
6158+
(list (neomacs--override-target)
6159+
(funcall 'neomacs--override-target)))
6160+
(neomacs--override-target)))"#
6161+
),
6162+
"OK (base (override override) base)"
6163+
);
6164+
}
6165+
6166+
#[test]
6167+
fn compiler_function_overrides_cache_tracks_default_assignment() {
6168+
crate::test_utils::init_test_tracing();
6169+
assert_eq!(
6170+
eval_one(
6171+
r#"(progn
6172+
(fset 'neomacs--default-override-target (lambda () 'base))
6173+
(set-default-toplevel-value
6174+
'internal--compiler-function-overrides
6175+
(list (cons 'neomacs--default-override-target
6176+
(lambda () 'override))))
6177+
(let ((during (list (neomacs--default-override-target)
6178+
(funcall 'neomacs--default-override-target))))
6179+
(set-default-toplevel-value
6180+
'internal--compiler-function-overrides nil)
6181+
(list during (neomacs--default-override-target))))"#
6182+
),
6183+
"OK ((override override) base)"
6184+
);
6185+
}
6186+
61476187
#[test]
61486188
fn funcall_builtin_wrong_arity_uses_subr_object_payload() {
61496189
crate::test_utils::init_test_tracing();

0 commit comments

Comments
 (0)