From 6a85b7930ed6332c03e793e675fb8d5969609ee0 Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Sat, 4 Jul 2026 15:06:17 +0200 Subject: [PATCH 1/2] [server] display: honor request-provided file contents over mtime-validated caches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unsaved editor buffers arrive as 'contents' in display requests (haxe-language-server sends them on every hover), but all three server cache layers validate freshness by disk mtime only, which does not change while the user types: - parse_file serves the cached disk parse on mtime match without consulting com.file_contents (and dms_per_file could cache a contents-parse keyed by disk mtime, poisoning later requests); - check_module deems the module clean (mtime unchanged), so the stale typed module is served and the display position is either never visited (empty hover) or resolved against old content (wrong-symbol hover); - check_display_file types the cached c_decls directly with no freshness validation at all. haxe-language-server sends server/invalidate on each edit, which evicts these caches and usually hides the problem. But nothing invalidates again when a compilation or a diagnostics run re-populates them from disk: with unsaved edits still pending, the next display request is answered from the freshly cached disk state instead of the buffer. Fix: request contents take priority. parse_file bypasses the parse cache (and skips caching) for files with provided contents; check_module compares the contents-parse against the cached declarations (not gated by NoFileSystemCheck — no file system involved); check_display_file re-parses and falls back to fresh typing when the declarations differ (needed even with the check_module fix: a display module already loaded fresh this request hits module_lut directly, skipping the hook). Regression test: cases/UnsavedContentsStale — hover carrying newer contents after a compile with no invalidate in between. Unfixed, it resolves the offset against the stale disk parse and returns the type of the identifier that previously occupied it (Int from the old `aaa` instead of String from the new `bbb`). --- src/compiler/server/serverCache.ml | 15 ++++++++++ src/context/display/displayTexpr.ml | 7 ++++- .../server/src/cases/UnsavedContentsStale.hx | 28 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/server/src/cases/UnsavedContentsStale.hx diff --git a/src/compiler/server/serverCache.ml b/src/compiler/server/serverCache.ml index 116ea9a55b4..13bcae380b2 100644 --- a/src/compiler/server/serverCache.ml +++ b/src/compiler/server/serverCache.ml @@ -14,9 +14,14 @@ let parse_file sctx com (rfile : ClassPaths.resolved_file) p = let ffile = Path.get_full_path rfile.file and fkey = com.part_scope.file_keys#get file in let is_display_file = DisplayPosition.display_position#is_in_file (com.part_scope.file_keys#get ffile) in + let has_request_contents = + com.file_contents <> [] && (try List.assoc fkey com.file_contents <> None with Not_found -> false) + in match is_display_file, sctx.ServerCompilationContext.current_stdin with | true, Some stdin when (com.file_contents <> [] || Common.defined com Define.DisplayStdin) -> TypeloadParse.parse_file_from_string com file p stdin + | _ when has_request_contents -> + TypeloadParse.parse_file com rfile p | _ -> let ftime = file_time ffile in let data = Std.finally (Timer.start_timer com.timer_ctx ["server";"parser cache"]) (fun () -> @@ -213,6 +218,15 @@ let check_module sctx com m_path m_extra p = end end in + let check_request_contents () = + let file = Path.UniqueKey.lazy_path m_extra.m_file in + let fkey = Path.UniqueKey.lazy_key m_extra.m_file in + let has_contents = (try List.assoc fkey com.file_contents <> None with Not_found -> false) in + if has_contents && content_changed m_path file then begin + ServerMessage.not_cached com "" m_path; + raise (Dirty (FileChanged file)) + end + in let find_module_extra sign mpath = (com.cs#get_context sign)#find_module_extra mpath in @@ -234,6 +248,7 @@ let check_module sctx com m_path m_extra p = try check_module_path(); if not (has_policy NoFileSystemCheck) || Path.file_extension (Path.UniqueKey.lazy_path m_extra.m_file) <> "hx" then check_file(); + if com.file_contents <> [] then check_request_contents(); if (get_typing_mode com m_extra) = FullTyping then check_dependencies(); None with diff --git a/src/context/display/displayTexpr.ml b/src/context/display/displayTexpr.ml index dc022a193b0..00cc14dd80b 100644 --- a/src/context/display/displayTexpr.ml +++ b/src/context/display/displayTexpr.ml @@ -166,8 +166,13 @@ let check_display_file ctx cs = | Some cc -> begin try let p = DisplayPosition.display_position#get in - let cfile = cc#find_file (ctx.com.part_scope.file_keys#get p.pfile) in + let fkey = ctx.com.part_scope.file_keys#get p.pfile in + let cfile = cc#find_file fkey in let path = (cfile.c_package,get_module_name_of_cfile p.pfile cfile) in + if (try List.assoc fkey ctx.com.file_contents <> None with Not_found -> false) then begin + let _,_,_,decls,_ = TypeloadParse.parse_module' ctx.com path null_pos in + if decls <> cfile.c_decls then raise Not_found + end; TypeloadParse.PdiHandler.handle_pdi ctx.com cfile.c_pdi; (* We have to go through type_module_hook because one of the module's dependencies could be invalid (issue #8991). *) diff --git a/tests/server/src/cases/UnsavedContentsStale.hx b/tests/server/src/cases/UnsavedContentsStale.hx new file mode 100644 index 00000000000..2ab6fd61c4c --- /dev/null +++ b/tests/server/src/cases/UnsavedContentsStale.hx @@ -0,0 +1,28 @@ +package cases; + +import haxe.display.FsPath; +import haxe.display.Server; +import TestCase; +import utest.Assert; + +// A hover carrying unsaved buffer contents must be answered against THOSE contents even when the +// server has a cached parse of the file whose disk mtime still matches (file not saved). Models +// vshaxe: edit -> (invalidate deduplicated away / compile re-cached the disk parse) -> hover. +class UnsavedContentsStale extends TestCase { + static inline var V1 = "class A {\n\tstatic function main() {\n\t\tvar aaa = 42;\n\t\ttrace(aaa);\n\t}\n}"; + static inline var V2 = "class A {\n\tstatic function main() {\n\t\tvar bbb = \"s\";\n\t\tvar aaa = 42;\n\t\ttrace(aaa);\n\t}\n}"; + + function test(_) { + vfs.putContent("A.hx", V1); + var args = ["-main", "A", "-js", "no.js", "--no-output"]; + runHaxe(args); + assertSuccess(); + + var offset = V1.indexOf("aaa") + 1; // == V2.indexOf("bbb") + 1 + + // NO invalidate: the compile above cached A's disk parse; hover sends newer unsaved contents. + var res = runHaxeJson(args, DisplayMethods.Hover, {file: new FsPath("A.hx"), offset: offset, contents: V2}); + var t = if (res == null || res.item == null) "" else try res.item.type.args.path.typeName catch (_) ""; + Assert.equals("String", t, "got " + t); + } +} From 39f1503367b633d1988f44cd6045fbb70e0c4917 Mon Sep 17 00:00:00 2001 From: Rudy Ges Date: Sat, 4 Jul 2026 15:17:32 +0200 Subject: [PATCH 2/2] [server] display: cut the cost of request-contents freshness checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two optimizations over the previous commit, measured on a pathological 351KB / 3000-decl display file (typical files are 10-30x smaller); without them the freshness checks re-parsed the buffer at every check site on every hover (~180ms at this size). - displayJson drops request contents that are byte-identical to the on-disk file (the common case: hover with no unsaved edits). The mtime-validated caches then stay fully usable. Clean-buffer hover: 35ms vs 33ms unfixed — the fix-attributable cost is one file read + string compare (~1ms even at this size); the ~22ms both pay over the 13ms no-contents baseline is JSON payload decode of the contents string, which pre-exists. - context_cache grows a tmp_parse_cache (cleared per request alongside tmp_binary_cache): the contents-parse is computed once and shared by check_module, check_display_file and module typing instead of re-parsing at each site. Dirty-buffer hover (unsaved edits pending): ~100ms at this size = one parse + full fresh display typing — the price of a correct answer where the unfixed server returned a wrong-symbol result. --- src/compiler/compilationCache.ml | 10 +++++++++- src/compiler/server/serverCache.ml | 7 ++++++- src/context/display/displayJson.ml | 8 ++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/compiler/compilationCache.ml b/src/compiler/compilationCache.ml index 610c970b62f..8e823dd7ed2 100644 --- a/src/compiler/compilationCache.ml +++ b/src/compiler/compilationCache.ml @@ -36,6 +36,7 @@ class context_cache (index : int) (sign : Digest.t) = object(self) val modules : (path,module_def) Hashtbl.t = Hashtbl.create 0 val binary_cache : (path,HxbData.module_cache) Hashtbl.t = Hashtbl.create 0 val tmp_binary_cache : (path,HxbData.module_cache) Hashtbl.t = Hashtbl.create 0 + val tmp_parse_cache : (Path.UniqueKey.t,(string list * type_decl list) Parser.parse_result) Hashtbl.t = Hashtbl.create 0 val get_hxb_module_mutex = Mutex.create () val removed_files = Hashtbl.create 0 val mutable json = JNull @@ -110,7 +111,14 @@ class context_cache (index : int) (sign : Digest.t) = object(self) Hashtbl.replace modules path m method clear_temp_cache = - Hashtbl.clear tmp_binary_cache + Hashtbl.clear tmp_binary_cache; + Hashtbl.clear tmp_parse_cache + + method find_tmp_parse key = + Hashtbl.find tmp_parse_cache key + + method cache_tmp_parse key r = + Hashtbl.replace tmp_parse_cache key r method clear_cache = Hashtbl.clear modules; diff --git a/src/compiler/server/serverCache.ml b/src/compiler/server/serverCache.ml index 13bcae380b2..d97c9b78a77 100644 --- a/src/compiler/server/serverCache.ml +++ b/src/compiler/server/serverCache.ml @@ -21,7 +21,12 @@ let parse_file sctx com (rfile : ClassPaths.resolved_file) p = | true, Some stdin when (com.file_contents <> [] || Common.defined com Define.DisplayStdin) -> TypeloadParse.parse_file_from_string com file p stdin | _ when has_request_contents -> - TypeloadParse.parse_file com rfile p + (try + cc#find_tmp_parse fkey + with Not_found -> + let r = TypeloadParse.parse_file com rfile p in + cc#cache_tmp_parse fkey r; + r) | _ -> let ftime = file_time ffile in let data = Std.finally (Timer.start_timer com.timer_ctx ["server";"parser cache"]) (fun () -> diff --git a/src/context/display/displayJson.ml b/src/context/display/displayJson.ml index 1e1971eb117..8e4c4f05dbd 100644 --- a/src/context/display/displayJson.ml +++ b/src/context/display/displayJson.ml @@ -88,6 +88,10 @@ class display_handler (jsonrpc : jsonrpc_handler) com (cs : CompilationCache.t) pmax = pos; }; + let contents = match contents with + | Some s when (try Std.input_file ~bin:true file = s with _ -> false) -> None + | c -> c + in com.file_contents <- [file_unique, contents]; end else begin let file_contents = jsonrpc#get_opt_param (fun () -> @@ -103,6 +107,10 @@ class display_handler (jsonrpc : jsonrpc_handler) com (cs : CompilationCache.t) let s = jsonrpc#get_string_field "fileContents" "contents" fl in Some s ) None in + let contents = match contents with + | Some s when (try Std.input_file ~bin:true file = s with _ -> false) -> None + | c -> c + in (file_unique, contents) | _ -> invalid_arg "fileContents" ) file_contents in