From 8f183c8c185f43db596b6aac1983f97403503592 Mon Sep 17 00:00:00 2001 From: Matthew Lugg Date: Sat, 17 Oct 2026 12:34:16 +0100 Subject: [PATCH] MappedFile: slight tweak to node deletion This strategy is a bit simpler and lets us reuse nodes more quickly, at the cost of a single bit (of which we had 17 spare anyway). --- src/link/Coff.zig | 2 +- src/link/Elf2.zig | 2 +- src/link/MachO2.zig | 1 + src/link/MappedFile.zig | 75 +++++++++++++++++++++++++++++------------ 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/link/Coff.zig b/src/link/Coff.zig index 5fede2640f4d8056ca5a5e1163b1a6a0ab1ea253..3b65bdb3b43d9ad78e496030ccf0ade0e864fbca 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -6100,7 +6100,7 @@ pub fn idle(coff: *Coff) !bool { break :task; } while (coff.mf.updates.pop()) |ni| : (coff.mf.update_prog_node.completeOne()) { - if (ni.pendingDelete(&coff.mf)) continue; + ni.cleanUpdated(&coff.mf); const clean_moved = ni.cleanMoved(&coff.mf); const clean_resized = ni.cleanResized(&coff.mf); const clean_next_moved = ni.cleanNextMoved(&coff.mf); diff --git a/src/link/Elf2.zig b/src/link/Elf2.zig index 2b68bb4683429019083d9cfccb3b05d991a054c4..0f7e44d1d559618370a0d032e2c40525a61b1e78 100644 --- a/src/link/Elf2.zig +++ b/src/link/Elf2.zig @@ -10857,7 +10857,7 @@ pub fn idle(elf: *Elf) link.Error!bool { break :task; } while (elf.mf.updates.pop()) |ni| : (elf.mf.update_prog_node.completeOne()) { - if (ni.pendingDelete(&elf.mf)) continue; + ni.cleanUpdated(&elf.mf); const clean_moved = ni.cleanMoved(&elf.mf); const clean_resized = ni.cleanResized(&elf.mf); const clean_next_moved = ni.cleanNextMoved(&elf.mf); diff --git a/src/link/MachO2.zig b/src/link/MachO2.zig index 96993fd0e9239572d1dbe04c295e51344009acd3..17e7d97d3c8916be857f5279bfde7607dc590134 100644 --- a/src/link/MachO2.zig +++ b/src/link/MachO2.zig @@ -3342,6 +3342,7 @@ pub fn idle(macho: *MachO) link.Error!bool { } while (macho.mf.updates.pop()) |ni| { + ni.cleanUpdated(&macho.mf); const clean_moved = ni.cleanMoved(&macho.mf); const clean_resized = ni.cleanResized(&macho.mf); const clean_next_moved = ni.cleanNextMoved(&macho.mf); diff --git a/src/link/MappedFile.zig b/src/link/MappedFile.zig index 5db69a96c00af9a0a6ad9fc0f987d5a37fab3b9e..e455ba232dc46dac3c3d54e8e57209486ccaa13e 100644 --- a/src/link/MappedFile.zig +++ b/src/link/MappedFile.zig @@ -207,6 +207,7 @@ pub fn init(file: Io.File, gpa: Allocator, io: Io) (Allocator.Error || Io.Cancel .resized = false, .next_moved = false, .has_content = false, + .updated = false, }, .location_payload = switch (root_location) { .small => |small| .{ .small = small }, @@ -287,7 +288,14 @@ pub const Node = extern struct { next_moved: bool, /// Whether this node might contain initialized bytes. has_content: bool, - unused: u17 = 0, + + /// Whether this node is currently in `updates`. Usually this is just equal to whether any + /// of `moved`, `resized`, or `next_moved` is set, but when a node is deleted and later + /// reused, it may still be in `updates`, so preserving this flag through the deletion + /// prevents us from adding it to `updates` a second time. + updated: bool, + + unused: u16 = 0, }; pub const Location = union(enum(u1)) { @@ -494,6 +502,11 @@ pub const Node = extern struct { } } + pub fn cleanUpdated(ni: Node.Index, mf: *MappedFile) void { + assert(ni.get(mf).flags.updated); + ni.get(mf).flags.updated = false; + } + pub fn hasMoved(ni: Node.Index, mf: *const MappedFile) bool { var cur_ni = ni; while (cur_ni != .root) { @@ -518,9 +531,11 @@ pub const Node = extern struct { if (node.prev.unwrap()) |prev_ni| prev_ni.nextMovedAssumeCapacity(mf); if (ni.hasMoved(mf)) return; node.flags.moved = true; - if (node.flags.resized or node.flags.next_moved) return; - mf.updates.appendAssumeCapacity(ni); - mf.update_prog_node.increaseEstimatedTotalItems(1); + if (!node.flags.updated) { + node.flags.updated = true; + mf.updates.appendAssumeCapacity(ni); + mf.update_prog_node.increaseEstimatedTotalItems(1); + } } pub fn hasResized(ni: Node.Index, mf: *const MappedFile) bool { @@ -539,9 +554,11 @@ pub const Node = extern struct { const node = ni.get(mf); if (node.flags.resized) return; node.flags.resized = true; - if (node.flags.moved or node.flags.next_moved) return; - mf.updates.appendAssumeCapacity(ni); - mf.update_prog_node.increaseEstimatedTotalItems(1); + if (!node.flags.updated) { + node.flags.updated = true; + mf.updates.appendAssumeCapacity(ni); + mf.update_prog_node.increaseEstimatedTotalItems(1); + } } pub fn hasNextMoved(ni: Node.Index, mf: *const MappedFile) bool { @@ -560,9 +577,11 @@ pub const Node = extern struct { const node = ni.get(mf); if (!node.flags.enable_next_moved or node.flags.next_moved) return; node.flags.next_moved = true; - if (node.flags.moved or node.flags.resized) return; - mf.updates.appendAssumeCapacity(ni); - mf.update_prog_node.increaseEstimatedTotalItems(1); + if (!node.flags.updated) { + node.flags.updated = true; + mf.updates.appendAssumeCapacity(ni); + mf.update_prog_node.increaseEstimatedTotalItems(1); + } } pub fn alignment(ni: Node.Index, mf: *const MappedFile) Alignment { @@ -647,19 +666,30 @@ pub const Node = extern struct { pub fn delete(ni: Node.Index, gpa: Allocator, mf: *MappedFile) Allocator.Error!void { const node = ni.get(mf); assert(node.first == .none and node.last == .none); // has children - mf.removeNodesFromChildList(gpa, ni, ni); - const updated = node.flags.moved or node.flags.resized or node.flags.next_moved; - node.* = undefined; - node.next = ni.toOptional(); - if (!updated) assert(ni.pendingDelete(mf)); - } - - pub fn pendingDelete(ni: Node.Index, mf: *MappedFile) bool { - const node = ni.get(mf); - if (node.next != ni.toOptional()) return false; - node.next = mf.free_ni; + try mf.removeNodesFromChildList(gpa, ni, ni); + node.* = .{ + .parent = undefined, + .prev = undefined, + .next = mf.free_ni, + .first = undefined, + .last = undefined, + .flags = .{ + .position = .floating, + .alignment = .@"1", + .bubbles_moved = false, + .enable_next_moved = false, + .location_tag = .small, + .moved = false, + .resized = false, + .next_moved = false, + .has_content = false, + // It is important that we preserve this flag! When the node gets reused, this + // prevents it being redundantly added to `mf.updates` if it is already there. + .updated = node.flags.updated, + }, + .location_payload = undefined, + }; mf.free_ni = ni.toOptional(); - return true; } /// Ensures that the size of `ni` is at least `min_size`. Valid for any node. @@ -946,6 +976,7 @@ fn addNode(mf: *MappedFile, gpa: Allocator, opts: struct { .resized = false, .next_moved = false, .has_content = false, + .updated = false, }, .location_payload = switch (location) { .small => |small| .{ .small = small }, -- 2.54.0