cleanup timing commit

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);
+}

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.

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;
 

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);