From 09b018ececb004d85faa30afa56f3eeb6b0fce42 Mon Sep 17 00:00:00 2001 From: copygirl Date: Wed, 13 Sep 2023 20:07:59 +0200 Subject: [PATCH] Make Path parts sentinel terminated This means Entity.init doesn't need to allocate. --- src/entity.zig | 22 ++++++---------------- src/path.zig | 45 ++++++++++++++++++++++----------------------- 2 files changed, 28 insertions(+), 39 deletions(-) diff --git a/src/entity.zig b/src/entity.zig index 2a05c77..b6e2aae 100644 --- a/src/entity.zig +++ b/src/entity.zig @@ -59,9 +59,9 @@ pub fn Entity(comptime ctx: anytype) type { // Set to modify an existing entity. id: ?Self = null, /// Set to specify the entity's name. - name: ?[]const u8 = null, + name: ?[:0]const u8 = null, /// Set to specify the entity's symbol. - symbol: ?[]const u8 = null, + symbol: ?[:0]const u8 = null, /// Whether to use a small id when generating one. /// These are typically reserved for components. use_low_id: bool = false, @@ -95,9 +95,6 @@ pub fn Entity(comptime ctx: anytype) type { var name = config.name; var scope = if (config.parent) |p| p.raw else null; - // TODO: Use an allocator that's well-fitted for super short-lived allocations. - const alloc = flecs.allocator; - if (config.path) |path| { if (path.absolute) { // Specifying parent with an absolute path is invalid. @@ -131,11 +128,9 @@ pub fn Entity(comptime ctx: anytype) type { scope = found; }, .name => |n| { - const nameZ = try alloc.dupeZ(u8, n); - defer alloc.free(nameZ); - const found = c.ecs_lookup_child(world.raw, parent, nameZ.ptr); + const found = c.ecs_lookup_child(world.raw, parent, n.ptr); if (found == 0) { - var desc = std.mem.zeroInit(c.ecs_entity_desc_t, .{ .sep = "".ptr, .name = nameZ.ptr }); + var desc = std.mem.zeroInit(c.ecs_entity_desc_t, .{ .sep = "".ptr, .name = n.ptr }); desc.add[0] = c.ecs_pair(c.EcsChildOf, parent); scope = c.ecs_entity_init(world.raw, &desc); if (scope == 0) return err.getLastErrorOrUnknown(); @@ -150,16 +145,11 @@ pub fn Entity(comptime ctx: anytype) type { const previous = if (scope) |s| world.setScope(s) else null; defer _ = if (scope != null) world.setScope(previous); - const nameZ = if (name) |n| try alloc.dupeZ(u8, n) else null; - defer if (nameZ) |n| alloc.free(n); - const symbolZ = if (config.symbol) |s| try alloc.dupeZ(u8, s) else null; - defer if (symbolZ) |s| alloc.free(s); - var desc = std.mem.zeroInit(c.ecs_entity_desc_t, .{ .sep = "".ptr, // Disable tokenization. .id = if (id) |i| i else 0, - .name = if (nameZ) |n| n.ptr else null, - .symbol = if (symbolZ) |s| s.ptr else null, + .name = if (name) |n| n.ptr else null, + .symbol = if (config.symbol) |s| s.ptr else null, .use_low_id = config.use_low_id, }); diff --git a/src/path.zig b/src/path.zig index e65a28f..04e2972 100644 --- a/src/path.zig +++ b/src/path.zig @@ -24,7 +24,7 @@ pub const Path = struct { /// Represents an `Entity` in a `Path`, either by name or its numeric id. pub const EntityPart = union(enum) { id: u32, - name: []const u8, + name: [:0]const u8, }; /// Format used to parse and stringify `Path`s. @@ -62,32 +62,29 @@ pub const Path = struct { /// to this function, as they aren't cloned and ownership stays the same. /// In many cases, the lifetime of `Path`s is relatively short. When this /// is not the case, it's recommended to `.clone()` the path after creation. - pub fn buildParts(parts: anytype) t: { - if (!std.meta.trait.isTuple(@TypeOf(parts))) + pub fn buildParts(parts: anytype) [numElements(parts)]EntityPart { + if (comptime !std.meta.trait.isTuple(@TypeOf(parts))) @compileError("Expected tuple, got '" ++ @typeName(@TypeOf(parts)) ++ "'"); - const len = @typeInfo(@TypeOf(parts)).Struct.fields.len; - break :t [len]EntityPart; - } { - const len = @typeInfo(@TypeOf(parts)).Struct.fields.len; - var result: [len]EntityPart = undefined; + var result: [numElements(parts)]EntityPart = undefined; inline for (&result, parts) |*res, part| { - const msg = "Expected '[]const u8' or 'u32', got '" ++ @typeName(@TypeOf(part)) ++ "'"; - res.* = switch (@typeInfo(@TypeOf(part))) { - .Pointer => |p| switch (p.size) { - .One => switch (@typeInfo(p.child)) { - .Array => |a| if (a.child == u8) .{ .name = part } else @compileError(msg), - else => @compileError(msg), - }, - .Slice => if (p.child == u8) .{ .name = part } else @compileError(msg), - else => @compileError(msg), - }, + res.* = if (comptime std.meta.trait.isZigString(@TypeOf(part))) + .{ .name = part } + else switch (@typeInfo(@TypeOf(part))) { .Int, .ComptimeInt => .{ .id = part }, - else => @compileError(msg), + else => @compileError("Expected '[:0]const u8' or 'u32', got '" ++ @typeName(@TypeOf(part)) ++ "'"), }; } return result; } + /// Returns the number of elements of the specified tuple type. + fn numElements(parts: anytype) usize { + return if (comptime std.meta.trait.isTuple(@TypeOf(parts))) + @typeInfo(@TypeOf(parts)).Struct.fields.len + else + 0; + } + /// Creates a new `Path` from the specified parts. /// /// The resulting path does not own any of the given slices. @@ -103,8 +100,9 @@ pub const Path = struct { /// resulting path will be absolute. The rest of the string will be split /// by the specified seperator, becoming its parts. /// - /// The parts array will be allocated with the specified `Allocator` and is - /// owned by the resulting path. `deinit()` must be called to free it. + /// This function will allocate duplicate strings taken from the specified + /// source `path`, to ensure they are sentinel-terminated. The resulting + /// `Path` takes ownership of these. pub fn fromString(path: []const u8, options: ?FormatOptions, alloc: Allocator) !Path { if (path.len == 0) return error.MustNotBeEmpty; const opt = options orelse FormatOptions.default; @@ -129,13 +127,14 @@ pub const Path = struct { parts[i] = if (parseNumericId(str)) |id| .{ .id = id } else - .{ .name = str }; + .{ .name = try alloc.dupeZ(u8, str) }; return .{ .absolute = absolute, .parts = parts, .alloc = alloc, .owns_array = true, + .owns_parts = true, }; } @@ -202,7 +201,7 @@ pub const Path = struct { for (parts) |*part| { if (part.* == .name) - part.* = .{ .name = try alloc.dupe(u8, part.name) }; + part.* = .{ .name = try alloc.dupeZ(u8, part.name) }; num_allocated += 1; }