Skip to content

binarytrees/1.zig Node allocator #408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

nomota
Copy link

@nomota nomota commented Jul 7, 2023

In this Zig code, every Node object had its own allocator. Assigning allocator to every Node object is absurd.

Removing allocator from Node struct would be a simple solution.

However simingly @hanabi1224 does not like that idea because Zig language usually assigns specific allocator to each data structure and it's fair to do so, reflecting the language's norm of its own standard library.

Fine. Zig language allows static members in struct.

So I let specific allocator still remain in Node structure, but it can be changed to be static. The allocator in Node now became static and set only once.

Assigning allocator to every Node object is absurd. Make it static, so that set only once.
@hanabi1224 hanabi1224 force-pushed the main branch 3 times, most recently from 3fc55d3 to d70acc7 Compare July 12, 2023 15:40
@highfestiva
Copy link

Sounds reasonable.

@rolfscherer
Copy link

I think a bufferd writer is also useful:

const stdout_file = std.io.getStdOut().writer(); var bw = std.io.bufferedWriter(stdout_file); const stdout = bw.writer();

An arena allocator should also be faster when releasing memory

@ivanstepanovftw
Copy link

ivanstepanovftw commented Jul 28, 2024

This is unfair to assign allocator to each node. Rust solution does not store a reference to C allocator free pointer in each node.

@ivanstepanovftw
Copy link

ivanstepanovftw commented Jun 30, 2025

It was so fun to read source code once again with a Git blame on.

  • @hanabi1224 Li Zhihong is putting into each node a copy of global allocator, effectively storing a pointer to C allocator Vtable. Why? This takes 1.5x more memory per node. In a memory-bound benchmark. binarytrees/1.zig Node allocator #408

  • Currently, Li Zhihong in his Zig solution is benchmarking both libc allocator and system calls. This affects users decisions in their programming language choice. For 2 years. zig binarytrees: use ArenaAllocator #417

  • We still cannot compare Zig to other languages performance for this test until this PR merged.

  • Rust has 5 solutions for this problem. Zig has 1. Thus said, Rust is second language choice for Li Zhihong after C#.

@cyrusmsk
Copy link
Contributor

Hey @hanabi1224
Do you have any concerns regarding this PR?

some Zig community people are upset to not having it merged.
If you will have time please check this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants