From 9ec9c0f5e57820f4baa04b9674a6a4a88235b863 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 19 Aug 2020 17:52:22 -0700 Subject: [PATCH] optimize the memory layout of Module.Fn and Module.Var `is_pub` added to `Fn` would cost us an additional 8 bytes of memory per function, which is a real bummer since it's only 1 bit of information. If we wanted to really remove this, I suspect we could make this a function isPub() which looks at the AST of the corresponding Decl and finds if the FnProto AST node has the pub token. However I saw an easier approach - The data of whether something is pub or not is actually a property of a Decl anyway, not a function, so we can look at moving the field into Decl. Indeed, doing this, we see that Decl already has deletion_flag: bool which is hiding in the padding bytes between the enum (1 byte) and the following u32 field (generation). So if we put the is_pub bool there, it actually will take up no additional space, with 1 byte of padding remaining. This was an easy reworking of the code since any func.is_pub could be changed simply to func.owner_decl.is_pub. I also modified `Var` to make the init value non-optional and moved the optional bit to a has_init: bool field. This is worse from the perspective of control flow and safety, however it makes `@sizeOf(Var)` go from 32 bytes to 24 bytes. The more code we can fit into memory at once, the more justified we are in using the compiler as a long-running process that does incremental updates. --- src-self-hosted/Module.zig | 26 +++++++++++++------------- src-self-hosted/zir.zig | 2 +- src-self-hosted/zir_sema.zig | 1 - test/stage2/compare_output.zig | 2 ++ 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src-self-hosted/Module.zig b/src-self-hosted/Module.zig index 8009bcc32d..ec69f94e3d 100644 --- a/src-self-hosted/Module.zig +++ b/src-self-hosted/Module.zig @@ -170,6 +170,9 @@ pub const Decl = struct { /// This flag is set when this Decl is added to a check_for_deletion set, and cleared /// when removed. deletion_flag: bool, + /// Whether the corresponding AST decl has a `pub` keyword. + is_pub: bool, + /// An integer that can be checked against the corresponding incrementing /// generation field of Module. This is used to determine whether `complete` status /// represents pre- or post- re-analysis. @@ -290,8 +293,6 @@ pub const Fn = struct { }, owner_decl: *Decl, - is_pub: bool, - /// This memory is temporary and points to stack memory for the duration /// of Fn analysis. pub const Analysis = struct { @@ -323,10 +324,10 @@ pub const Fn = struct { }; pub const Var = struct { - value: ?Value, + init: Value, owner_decl: *Decl, - is_pub: bool, + has_init: bool, is_extern: bool, is_mutable: bool, is_threadlocal: bool, @@ -1247,7 +1248,7 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { }; defer fn_type_scope.instructions.deinit(self.gpa); - const is_pub = fn_proto.getTrailer("visib_token") != null; + decl.is_pub = fn_proto.getTrailer("visib_token") != null; const body_node = fn_proto.getTrailer("body_node") orelse return self.failTok(&fn_type_scope.base, fn_proto.fn_token, "TODO implement extern functions", .{}); @@ -1385,7 +1386,6 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { new_func.* = .{ .analysis = .{ .queued = fn_zir }, .owner_decl = decl, - .is_pub = is_pub, }; fn_payload.* = .{ .func = new_func }; @@ -1452,7 +1452,7 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { }; defer block_scope.instructions.deinit(self.gpa); - const is_pub = var_decl.getTrailer("visib_token") != null; + decl.is_pub = var_decl.getTrailer("visib_token") != null; const is_extern = blk: { const maybe_extern_token = var_decl.getTrailer("extern_export_token") orelse break :blk false; @@ -1477,7 +1477,6 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { return self.failNode(&block_scope.base, sect_expr, "TODO implement function section expression", .{}); } - const explicit_type = blk: { const type_node = var_decl.getTrailer("type_node") orelse break :blk null; @@ -1568,9 +1567,9 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { const new_variable = try decl_arena.allocator.create(Var); const var_payload = try decl_arena.allocator.create(Value.Payload.Variable); new_variable.* = .{ - .value = value, .owner_decl = decl, - .is_pub = is_pub, + .init = value orelse undefined, + .has_init = value != null, .is_extern = is_extern, .is_mutable = is_mutable, .is_threadlocal = is_threadlocal, @@ -2004,6 +2003,7 @@ fn allocateNewDecl( .wasm => .{ .wasm = null }, }, .generation = 0, + .is_pub = false, }; return new_decl; } @@ -2440,15 +2440,15 @@ fn analyzeVarRef(self: *Module, scope: *Scope, src: usize, tv: TypedValue) Inner const variable = tv.val.cast(Value.Payload.Variable).?.variable; const ty = try self.singlePtrType(scope, src, variable.is_mutable, tv.ty); - if (!variable.is_mutable and !variable.is_extern and variable.value != null) { + if (!variable.is_mutable and !variable.is_extern and variable.has_init) { const val_payload = try scope.arena().create(Value.Payload.RefVal); - val_payload.* = .{ .val = variable.value.? }; + val_payload.* = .{ .val = variable.init }; return self.constInst(scope, src, .{ .ty = ty, .val = Value.initPayload(&val_payload.base), }); } - + const b = try self.requireRuntimeBlock(scope, src); const inst = try b.arena.create(Inst.VarPtr); inst.* = .{ diff --git a/src-self-hosted/zir.zig b/src-self-hosted/zir.zig index 384463caee..552e90956b 100644 --- a/src-self-hosted/zir.zig +++ b/src-self-hosted/zir.zig @@ -1881,7 +1881,7 @@ const EmitZIR = struct { } else if (typed_value.val.cast(Value.Payload.Variable)) |variable| { return self.emitTypedValue(src, .{ .ty = typed_value.ty, - .val = variable.variable.value.?, + .val = variable.variable.init, }); } if (typed_value.val.isUndef()) { diff --git a/src-self-hosted/zir_sema.zig b/src-self-hosted/zir_sema.zig index 87cce3adb3..cb1ed08d6d 100644 --- a/src-self-hosted/zir_sema.zig +++ b/src-self-hosted/zir_sema.zig @@ -666,7 +666,6 @@ fn analyzeInstFn(mod: *Module, scope: *Scope, fn_inst: *zir.Inst.Fn) InnerError! new_func.* = .{ .analysis = .{ .queued = fn_zir }, .owner_decl = scope.decl().?, - .is_pub = false, }; const fn_payload = try scope.arena().create(Value.Payload.Function); fn_payload.* = .{ .func = new_func }; diff --git a/test/stage2/compare_output.zig b/test/stage2/compare_output.zig index 2b8c8e94cc..6fbc760f26 100644 --- a/test/stage2/compare_output.zig +++ b/test/stage2/compare_output.zig @@ -586,6 +586,7 @@ pub fn addCases(ctx: *TestContext) !void { "", ); + // Character literals and multiline strings. case.addCompareOutput( \\export fn _start() noreturn { \\ const ignore = @@ -618,6 +619,7 @@ pub fn addCases(ctx: *TestContext) !void { "", ); + // Global const. case.addCompareOutput( \\export fn _start() noreturn { \\ add(aa, bb);