From 418068a3d1d1772339c0fa2fae194cf18ae8e6f2 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Tue, 11 Aug 2026 09:59:51 -0700 Subject: [PATCH] cleanup timing commit --- src/Config.zig | 40 ++++++++++++++++++++++++++++++++++++++++ src/PortfolioData.zig | 17 ++++------------- src/portfolio_loader.zig | 25 ++++++++++++------------- src/service.zig | 6 +----- 4 files changed, 57 insertions(+), 31 deletions(-) diff --git a/src/Config.zig b/src/Config.zig index 7593510..85dc297 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -77,6 +77,17 @@ server_url: ?[]const u8 = null, /// requests then go out unauthenticated, which is correct against an open /// server or one still in its pre-enforcement soft cutover. server_api_key: ?[]const u8 = null, +/// Is startup/phase timing instrumentation requested (`ZFIN_TIMING`)? +/// +/// Resolved once here rather than re-parsed per call site. Two callers - +/// `PortfolioData` and `DataService.loadAllPrices` - each grew their own copy of +/// the same predicate in one commit, which is the drift this field prevents. +/// +/// Any non-empty value except "0" enables it. Instrumentation is emitted at +/// `info`, not `debug`, because release builds compile debug logging out - an +/// instrument that vanishes in the build people install is no instrument - so +/// this flag is what keeps a normal run silent. +timing: bool = false, cache_dir: []const u8, cache_dir_owned: bool = false, // true when cache_dir was allocated via path.join zfin_home: ?[]const u8 = null, @@ -101,6 +112,17 @@ environ_map: ?*const std.process.Environ.Map = null, // ── Construction / teardown ────────────────────────────────── +/// Truthiness of an environment flag: absent, empty and "0" are off, anything +/// else is on. +/// +/// Named rather than inlined so the one definition is also the one a test can +/// call. The inline version of this predicate got copied into two call sites in +/// a single commit. +fn envFlag(v: ?[]const u8) bool { + const s = v orelse return false; + return s.len > 0 and !std.mem.eql(u8, s, "0"); +} + pub fn fromEnv(io: std.Io, allocator: std.mem.Allocator, environ_map: *const std.process.Environ.Map) @This() { var self = @This(){ // SAFETY: assigned unconditionally below (the `cache_dir = @@ -143,6 +165,7 @@ pub fn fromEnv(io: std.Io, allocator: std.mem.Allocator, environ_map: *const std self.user_email = self.resolve("ZFIN_USER_EMAIL"); self.server_url = self.resolve("ZFIN_SERVER"); self.server_api_key = self.resolve("ZFIN_SERVER_API_KEY"); + self.timing = envFlag(self.resolve("ZFIN_TIMING")); const env_cache = self.resolve("ZFIN_CACHE_DIR"); self.cache_dir = env_cache orelse blk: { @@ -1016,3 +1039,20 @@ test "expandGlob: missing directory returns null" { const result = try expandGlob(io, allocator, "/zfin-test-no-such-dir-xyz", "*.srf", .home_relative); try testing.expect(result == null); } + +test "envFlag: absent, empty and \"0\" are off; anything else is on" { + // Two callers - PortfolioData and DataService.loadAllPrices - each grew an + // identical copy of this predicate in a single commit. One definition, and + // the test calls it rather than restating it. + try testing.expect(!envFlag(null)); + try testing.expect(!envFlag("")); + try testing.expect(!envFlag("0")); + for ([_][]const u8{ "1", "true", "yes", " ", "00" }) |on| { + try testing.expect(envFlag(on)); + } +} + +test "timing: defaults off so a normal run stays silent" { + const d: @This() = .{ .cache_dir = "unused" }; + try testing.expect(!d.timing); +} diff --git a/src/PortfolioData.zig b/src/PortfolioData.zig index 937a20f..d33fcca 100644 --- a/src/PortfolioData.zig +++ b/src/PortfolioData.zig @@ -522,20 +522,11 @@ fn awaitWorker(self: *PortfolioData, fut: *?std.Io.Future(void)) void { } } -/// Is startup timing instrumentation requested? -/// -/// Gated on an env var and emitted at `info` rather than `debug`, because the -/// release builds people actually install compile debug logging out entirely - -/// which made the first version of this instrumentation invisible in precisely -/// the build that was slow. Off by default, so a normal run stays silent. -fn timingOn(self: *PortfolioData) bool { - const em = self.svc.config.environ_map orelse return false; - const v = em.get("ZFIN_TIMING") orelse return false; - return v.len > 0 and !std.mem.eql(u8, v, "0"); -} - fn timing(self: *PortfolioData, comptime fmt: []const u8, args: anytype) void { - if (!self.timingOn()) return; + // `Config.timing` resolves ZFIN_TIMING once at init; this used to re-parse + // the env var here, with an identical copy of the predicate in + // `DataService.loadAllPrices`. + if (!self.svc.config.timing) return; // `log.info`, not `log.debug`: release builds compile debug logging out, and // an instrument that vanishes in the build people install is no instrument. // Gated on the env var, so a normal run stays silent either way. diff --git a/src/portfolio_loader.zig b/src/portfolio_loader.zig index ce263ce..78402fd 100644 --- a/src/portfolio_loader.zig +++ b/src/portfolio_loader.zig @@ -587,22 +587,21 @@ pub fn enrichLotsSplits( // with no output before first paint. Narrowing the corpus to the opt-in set // cuts that from every held symbol to the handful that asked for it. // - // Borrowing `cutovers`' keys is safe: `corpus` is torn down by the defer + // Borrowing `syms`' entries is safe: `corpus` is torn down by the defer // below, which runs before `freeCutovers` above it. var wanted: std.ArrayList([]const u8) = .empty; defer wanted.deinit(allocator); - { - var it = cutovers.keyIterator(); - while (it.next()) |k| { - // Only symbols actually held - `syms` is the held set, and a - // metadata row can name a symbol the portfolio no longer holds. - for (syms) |s| { - if (std.mem.eql(u8, s, k.*)) { - wanted.append(allocator, k.*) catch return; - break; - } - } - } + for (syms) |s| { + // Only symbols actually held - `syms` is the held set, and a metadata + // row can name a symbol the portfolio no longer holds. + // + // Driven from `syms` with a hash lookup, not from the map's keys with a + // linear scan of `syms` per key: same result, O(n) instead of O(n*m), + // and the output order follows the held set instead of hash iteration + // order. + // Bailing on OOM rather than propagating: split adjustment is opt-in + // enrichment, and the un-adjusted portfolio is still correct. + if (cutovers.contains(s)) wanted.append(allocator, s) catch return; } if (wanted.items.len == 0) return; diff --git a/src/service.zig b/src/service.zig index 3acd6ca..70a2861 100644 --- a/src/service.zig +++ b/src/service.zig @@ -2248,11 +2248,7 @@ pub const DataService = struct { // on its own does not distinguish a slow disk from a dead server. const t0 = std.Io.Timestamp.now(self.io, .real); var mark = t0; - const timing_on = blk: { - const em = self.config.environ_map orelse break :blk false; - const v = em.get("ZFIN_TIMING") orelse break :blk false; - break :blk v.len > 0 and !std.mem.eql(u8, v, "0"); - }; + const timing_on = self.config.timing; const Sub = struct { fn done(on: bool, io: std.Io, m: *std.Io.Timestamp, name: []const u8, n: usize) void { const now = std.Io.Timestamp.now(io, .real);