From e9e9571fe57ebeef78772c3db1181484a88d5771 Mon Sep 17 00:00:00 2001 From: Techatrix <19954306+Techatrix@users.noreply.github.com> Date: Thu, 22 Sep 2022 01:31:48 +0000 Subject: [PATCH] avoid config copy for document store (#669) --- src/DocumentStore.zig | 7 +++---- src/Server.zig | 29 +++++++++++------------------ src/inlay_hints.zig | 4 ++-- src/main.zig | 4 +++- src/zls.zig | 1 + tests/context.zig | 36 ++++++++++++++++++++++++------------ 6 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/DocumentStore.zig b/src/DocumentStore.zig index 9af90e8..6d685ee 100644 --- a/src/DocumentStore.zig +++ b/src/DocumentStore.zig @@ -82,8 +82,7 @@ allocator: std.mem.Allocator, handles: UriToHandleMap = .{}, build_files: BuildFileList = .{}, -// TODO use pointer back to Server.config -config: Config, +config: *Config, std_uri: ?[]const u8, // TODO make this configurable // We can't figure it out ourselves since we don't know what arguments @@ -95,7 +94,7 @@ zig_global_cache_root: []const u8 = "ZLS_DONT_CARE", pub fn init( allocator: std.mem.Allocator, - config: Config, + config: *Config, ) !DocumentStore { return DocumentStore{ .allocator = allocator, @@ -671,7 +670,7 @@ fn translate(self: *DocumentStore, handle: *Handle, source: []const u8) error{Ou const maybe_result = try translate_c.translate( self.allocator, - self.config, + self.config.*, include_dirs, source, ); diff --git a/src/Server.zig b/src/Server.zig index 42587fe..4c178bb 100644 --- a/src/Server.zig +++ b/src/Server.zig @@ -23,7 +23,7 @@ const log = std.log.scoped(.server); // Server fields -config: Config, +config: *Config, allocator: std.mem.Allocator = undefined, arena: std.heap.ArenaAllocator = undefined, document_store: DocumentStore = undefined, @@ -1164,7 +1164,7 @@ fn completeBuiltin(server: *Server, writer: anytype, id: types.RequestId) !void if (server.builtin_completions == null) { server.builtin_completions = try std.ArrayListUnmanaged(types.CompletionItem).initCapacity(server.allocator, data.builtins.len); - try populateBuiltinCompletions(&server.builtin_completions.?, server.config); + try populateBuiltinCompletions(&server.builtin_completions.?, server.config.*); } try send(writer, server.arena.allocator(), types.Response{ @@ -2054,7 +2054,7 @@ fn didChangeConfigurationHandler(server: *Server, writer: anytype, id: types.Req } } - try server.configChanged(null); + try server.config.configChanged(server.allocator, null); } else if (server.client_capabilities.supports_configuration) try server.requestConfiguration(writer); } @@ -2210,7 +2210,7 @@ fn inlayHintHandler(server: *Server, writer: anytype, id: types.RequestId, req: // with caching it would also make sense to generate all hints instead of only the visible ones const hints = try inlay_hints.writeRangeInlayHint( &server.arena, - &server.config, + server.config.*, &server.document_store, handle, req.params.range, @@ -2318,7 +2318,7 @@ pub fn processJsonRpc(server: *Server, writer: anytype, json: []const u8) !void } } - try server.configChanged(null); + try server.config.configChanged(server.allocator, null); return; } @@ -2425,14 +2425,9 @@ pub fn processJsonRpc(server: *Server, writer: anytype, json: []const u8) !void log.debug("Method without return value not implemented: {s}", .{method}); } -pub fn configChanged(server: *Server, builtin_creation_dir: ?[]const u8) !void { - try server.config.configChanged(server.allocator, builtin_creation_dir); - server.document_store.config = server.config; -} - pub fn init( allocator: std.mem.Allocator, - config: Config, + config: *Config, config_path: ?[]const u8, ) !Server { // TODO replace global with something like an Analyser struct @@ -2440,14 +2435,15 @@ pub fn init( // see: https://github.com/zigtools/zls/issues/536 analysis.init(allocator); - var cfg = config; + try config.configChanged(allocator, config_path); - try cfg.configChanged(allocator, config_path); + var document_store = try DocumentStore.init(allocator, config); + errdefer document_store.deinit(); return Server{ - .config = cfg, + .config = config, .allocator = allocator, - .document_store = try DocumentStore.init(allocator, cfg), + .document_store = document_store, }; } @@ -2455,9 +2451,6 @@ pub fn deinit(server: *Server) void { server.document_store.deinit(); analysis.deinit(); - const config_parse_options = std.json.ParseOptions{ .allocator = server.allocator }; - defer std.json.parseFree(Config, server.config, config_parse_options); - if (server.builtin_completions) |*compls| { compls.deinit(server.allocator); } diff --git a/src/inlay_hints.zig b/src/inlay_hints.zig index 4fb6f1c..032f2ff 100644 --- a/src/inlay_hints.zig +++ b/src/inlay_hints.zig @@ -660,7 +660,7 @@ fn writeNodeInlayHint(builder: *Builder, arena: *std.heap.ArenaAllocator, store: /// `InlayHint.tooltip.value` has to deallocated separately pub fn writeRangeInlayHint( arena: *std.heap.ArenaAllocator, - config: *const Config, + config: Config, store: *DocumentStore, handle: *DocumentStore.Handle, range: types.Range, @@ -669,7 +669,7 @@ pub fn writeRangeInlayHint( ) error{OutOfMemory}![]types.InlayHint { var builder: Builder = .{ .allocator = arena.child_allocator, - .config = config, + .config = &config, .handle = handle, .hints = .{}, .hover_kind = hover_kind, diff --git a/src/main.zig b/src/main.zig index e9f6f2d..9beecab 100644 --- a/src/main.zig +++ b/src/main.zig @@ -264,13 +264,15 @@ pub fn main() !void { } config = try getConfig(allocator, config.config_path, true); + defer std.json.parseFree(Config, config.config, .{ .allocator = allocator }); + if (config.config_path == null) { logger.info("No config file zls.json found.", .{}); } var server = try Server.init( allocator, - config.config, + &config.config, config.config_path, ); defer server.deinit(); diff --git a/src/zls.zig b/src/zls.zig index 2f89d96..c896309 100644 --- a/src/zls.zig +++ b/src/zls.zig @@ -3,6 +3,7 @@ pub const analysis = @import("analysis.zig"); pub const header = @import("header.zig"); pub const offsets = @import("offsets.zig"); pub const requests = @import("requests.zig"); +pub const Config = @import("Config.zig"); pub const Server = @import("Server.zig"); pub const translate_c = @import("translate_c.zig"); pub const types = @import("types.zig"); diff --git a/tests/context.zig b/tests/context.zig index 5629a4b..cff28e8 100644 --- a/tests/context.zig +++ b/tests/context.zig @@ -2,6 +2,7 @@ const std = @import("std"); const zls = @import("zls"); const headerPkg = zls.header; +const Config = zls.Config; const Server = zls.Server; const types = zls.types; const requests = zls.requests; @@ -10,25 +11,33 @@ const initialize_msg = \\{"processId":6896,"clientInfo":{"name":"vscode","version":"1.46.1"},"rootPath":null,"rootUri":null,"capabilities":{"workspace":{"applyEdit":true,"workspaceEdit":{"documentChanges":true,"resourceOperations":["create","rename","delete"],"failureHandling":"textOnlyTransactional"},"didChangeConfiguration":{"dynamicRegistration":true},"didChangeWatchedFiles":{"dynamicRegistration":true},"symbol":{"dynamicRegistration":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]},"tagSupport":{"valueSet":[1]}},"executeCommand":{"dynamicRegistration":true},"configuration":true,"workspaceFolders":true},"textDocument":{"publishDiagnostics":{"relatedInformation":true,"versionSupport":false,"tagSupport":{"valueSet":[1,2]},"complexDiagnosticCodeSupport":true},"synchronization":{"dynamicRegistration":true,"willSave":true,"willSaveWaitUntil":true,"didSave":true},"completion":{"dynamicRegistration":true,"contextSupport":true,"completionItem":{"snippetSupport":true,"commitCharactersSupport":true,"documentationFormat":["markdown","plaintext"],"deprecatedSupport":true,"preselectSupport":true,"tagSupport":{"valueSet":[1]},"insertReplaceSupport":true},"completionItemKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]}},"hover":{"dynamicRegistration":true,"contentFormat":["markdown","plaintext"]},"signatureHelp":{"dynamicRegistration":true,"signatureInformation":{"documentationFormat":["markdown","plaintext"],"parameterInformation":{"labelOffsetSupport":true}},"contextSupport":true},"definition":{"dynamicRegistration":true,"linkSupport":true},"references":{"dynamicRegistration":true},"documentHighlight":{"dynamicRegistration":true},"documentSymbol":{"dynamicRegistration":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]},"hierarchicalDocumentSymbolSupport":true,"tagSupport":{"valueSet":[1]}},"codeAction":{"dynamicRegistration":true,"isPreferredSupport":true,"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["","quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"codeLens":{"dynamicRegistration":true},"formatting":{"dynamicRegistration":true},"rangeFormatting":{"dynamicRegistration":true},"onTypeFormatting":{"dynamicRegistration":true},"rename":{"dynamicRegistration":true,"prepareSupport":true},"documentLink":{"dynamicRegistration":true,"tooltipSupport":true},"typeDefinition":{"dynamicRegistration":true,"linkSupport":true},"implementation":{"dynamicRegistration":true,"linkSupport":true},"colorProvider":{"dynamicRegistration":true},"foldingRange":{"dynamicRegistration":true,"rangeLimit":5000,"lineFoldingOnly":true},"declaration":{"dynamicRegistration":true,"linkSupport":true},"selectionRange":{"dynamicRegistration":true},"semanticTokens":{"dynamicRegistration":true,"tokenTypes":["comment","keyword","number","regexp","operator","namespace","type","struct","class","interface","enum","typeParameter","function","member","macro","variable","parameter","property","label"],"tokenModifiers":["declaration","documentation","static","abstract","deprecated","readonly"]}},"window":{"workDoneProgress":true}},"trace":"off","workspaceFolders":[{"uri":"file://./tests", "name":"root"}]} ; +const default_config: Config = .{ + .enable_ast_check_diagnostics = false, + .enable_semantic_tokens = true, + .enable_inlay_hints = true, + .inlay_hints_exclude_single_argument = false, + .inlay_hints_show_builtin = true, +}; + const allocator = std.testing.allocator; pub const Context = struct { server: Server, + config: *Config, request_id: u32 = 1, pub fn init() !Context { - var context = Context{ - .server = try Server.init( - allocator, - .{ - .enable_ast_check_diagnostics = false, - .enable_semantic_tokens = true, - .enable_inlay_hints = true, - .inlay_hints_exclude_single_argument = false, - .inlay_hints_show_builtin = true, - }, - null, - ), + var config = try allocator.create(Config); + errdefer allocator.destroy(config); + + config.* = default_config; + + var server = try Server.init(allocator, config, null); + errdefer server.deinit(); + + var context: Context = .{ + .server = server, + .config = config, }; try context.request("initialize", initialize_msg, null); @@ -37,6 +46,9 @@ pub const Context = struct { } pub fn deinit(self: *Context) void { + std.json.parseFree(Config, self.config.*, .{ .allocator = allocator }); + allocator.destroy(self.config); + self.request("shutdown", "{}", null) catch {}; self.server.deinit(); }