Data being trash after putting into `std.StringHashMap` | Zig Programming Language | Page 1

spice merlin Jan 19, 2024, 7:48 AM

#

I have MemoryStorage struct and can't retrive key from it, I set the data from socket by commands and then put into it, I can clearly see there is something for that key I set but I cannot retrive that properly.

Struct code: https://github.com/sectasy0/zcached/blob/master/src/server/storage.zig#L23

Thist "test" string after printing keys from my screenshot comes from get command, I printed key

GitHub

zcached/src/server/storage.zig at master · sectasy0/zcached

Lightweight and efficient in-memory caching system akin to databases like Redis. - sectasy0/zcached

spice merlin Jan 19, 2024, 8:47 AM

#

Code I used to print keys:

// Debug print storage entries
var iter = self.storage.internal.iterator();
while (iter.next()) |item| {
  std.debug.print("{s}\n", .{item.key_ptr.*});
}

#

Maybe I forgot to cast or something? but why is not finding in struct since is exact the same string

#

I think its related to lifetime, key becomes invalid after the request is completed, and because my tests works where everything is in one context

#

I have to copy somehow to another place where it lives long enough

ornate whale Jan 19, 2024, 9:06 AM

#

I used to have the same problem with the arraylist too, at first I was trying to appends slices of absolute_file_path using the std.fs.IterableDir or something like that. And just like you I wasn't getting anything just garbage. I switched from ArrayLIst([]const u8) to ArrayList(u8) and it worked so my guess it's probably a lifetime issue just like you said

#

I was probably copying the pointer instead of the actual values

spice merlin Jan 19, 2024, 9:21 AM

#

I'm not sure how to copy the value and then put in in StringHashMap

ornate whale Jan 19, 2024, 9:25 AM

#

I'm not sure either, but I've yet to use StringHashMap, can you show where you are adding the alues to the hash map ?

spice merlin Jan 19, 2024, 9:33 AM

#

spice merlin I have `MemoryStorage` struct and can't retrive key from it, I set the data from...

Here it is

ornate whale Jan 19, 2024, 9:36 AM

#

/// Builtin hashmap for strings as keys.
/// Key memory is managed by the caller.  Keys and values
/// will not automatically be freed.
pub fn StringHashMap(comptime V: type) type {
    return HashMap([]const u8, V, StringContext, default_max_load_percentage);
}

#

If I understand correctly it seems like you are responsible for the memory of the keys, so it probably just take a pointer to your string and doesn't copy it is my guess ?

spice merlin Jan 19, 2024, 9:37 AM

#

yeah

ornate whale Jan 19, 2024, 9:37 AM

#

so you probably need to eitheir store it somewhere on the heap, or use a different hashMap ?

spice merlin Jan 19, 2024, 9:38 AM

#

In my specific problem i clear all the memory i allocated in ArenaAllocator after request being finished

#

So the key is not valid after that, when i want to receive that key in separated request

ornate whale Jan 19, 2024, 9:38 AM

#

Yes this invalidates the pointer

spice merlin Jan 19, 2024, 9:39 AM

#

So i just have to copy those data in my put method

ornate whale Jan 19, 2024, 9:40 AM

#

yes also it might be worthwile to get more info to use the GeneralPurposeAllocator in conjunction with the loggingAllocator ? such that you can get more infos of where the memory allocations are happening etc

#

there is also that in the std.debug.Trace

/// This API helps you track where a value originated and where it was mutated,
/// or any other points of interest.
/// In debug mode, it adds a small size penalty (104 bytes on 64-bit architectures)
/// to the aggregate that you add it to.
/// In release mode, it is size 0 and all methods are no-ops.
/// This is a pre-made type with default settings.
/// For more advanced usage, see `ConfigurableTrace`.
pub const Trace = ConfigurableTrace(2, 4, builtin.mode == .Debug);

it might be worthwhile to embed this into your struct for tracing values ?

spice merlin Jan 19, 2024, 9:59 AM

#

Damn it segfaults when i try to copy with mem.cpy

switch (value) {
    AnyType.err => return error.CantInsertError,
    else => {
        // everything fucks up when i try this way
        std.debug.print("key before: {s}\n", .{key});
        var zkey: []u8 = try tracking.allocator().alloc(u8, key.len);
        std.mem.copy(u8, zkey, key);

        // var zvalue: *AnyType = try tracking.allocator().create(@TypeOf(value));
        // zvalue.* = value;
        //
        std.debug.print("{s} {any} \n", .{ zkey, value });

        self.internal.put(zkey, value) catch return error.InsertFailure;
    },
}

spice merlin Jan 19, 2024, 10:00 AM

#

ornate whale there is also that in the std.debug.Trace ``` /// This API helps you track wher...

I should use that

ornate whale Jan 19, 2024, 10:03 AM

#

I'm downloading your code to see

ornate whale Jan 19, 2024, 10:19 AM

#

Well i had to fix all the const stuff because I'm runing 0.12 but with the logging allocator i get that so far how can I test it more ?

#

debug: alloc - success - len: 20, ptr_align: 0
debug: alloc - success - len: 64, ptr_align: 3
debug: alloc - success - len: 26, ptr_align: 3
debug: free - len: 64
debug: free - len: 20
INFO [2024-01-19T11:18:05.000 CET] logger initialized, log_path: ./log/zcached.log
INFO [2024-01-19T11:18:05.000 CET] loading config file from: ./zcached.conf
debug: alloc - success - len: 224, ptr_align: 3
debug: alloc - success - len: 424, ptr_align: 3
debug: free - len: 224
debug: alloc - success - len: 64, ptr_align: 3
INFO [2024-01-19T11:18:05.000 CET] # starting zcached server on 127.0.0.1:7556
INFO [2024-01-19T11:18:05.000 CET] * ready to accept connections

#

Ok I've found a way here's what I get so far :

debug: alloc - success - len: 20, ptr_align: 0
debug: alloc - success - len: 64, ptr_align: 3
debug: alloc - success - len: 26, ptr_align: 3
debug: free - len: 64
debug: free - len: 20
INFO [2024-01-19T11:22:25.000 CET] logger initialized, log_path: ./log/zcached.log
INFO [2024-01-19T11:22:25.000 CET] loading config file from: ./zcached.conf
debug: alloc - success - len: 224, ptr_align: 3
debug: alloc - success - len: 424, ptr_align: 3
debug: free - len: 224
debug: alloc - success - len: 64, ptr_align: 3
INFO [2024-01-19T11:22:25.000 CET] # starting zcached server on 127.0.0.1:7556
INFO [2024-01-19T11:22:25.000 CET] * ready to accept connections
INFO [2024-01-19T11:23:19.000 CET] * new connection from 127.0.0.1:59127
debug: alloc - success - len: 144, ptr_align: 3
debug: alloc - success - len: 224, ptr_align: 3
debug: alloc - success - len: 424, ptr_align: 3
debug: free - len: 224
Bus error at address 0x16f7b4000
Panicked during a panic. Aborting.
[1]    65672 abort      ./zcached

#

with the verbose option enabled on the gpa I get :

info(gpa): small alloc 20 bytes at u8@1024f8000
debug: alloc - success - len: 20, ptr_align: 0
info(gpa): small alloc 64 bytes at u8@102508000
debug: alloc - success - len: 64, ptr_align: 3
info(gpa): small alloc 26 bytes at u8@1024f8020
debug: alloc - success - len: 26, ptr_align: 3
info(gpa): small free 64 bytes at u8@102508000
debug: free - len: 64
info(gpa): small free 20 bytes at u8@1024f8000
debug: free - len: 20
INFO [2024-01-19T11:25:04.000 CET] logger initialized, log_path: ./log/zcached.log
INFO [2024-01-19T11:25:04.000 CET] loading config file from: ./zcached.conf
info(gpa): small alloc 224 bytes at u8@102514000
debug: alloc - success - len: 224, ptr_align: 3
info(gpa): small alloc 424 bytes at u8@10251c000
debug: alloc - success - len: 424, ptr_align: 3
info(gpa): small free 224 bytes at u8@102514000
debug: free - len: 224
info(gpa): small alloc 64 bytes at u8@102524000
debug: alloc - success - len: 64, ptr_align: 3
INFO [2024-01-19T11:25:04.000 CET] # starting zcached server on 127.0.0.1:7556
INFO [2024-01-19T11:25:04.000 CET] * ready to accept connections
INFO [2024-01-19T11:25:11.000 CET] * new connection from 127.0.0.1:59136
info(gpa): small alloc 144 bytes at u8@102530000
debug: alloc - success - len: 144, ptr_align: 3
info(gpa): small alloc 224 bytes at u8@102530100
debug: alloc - success - len: 224, ptr_align: 3
info(gpa): small alloc 424 bytes at u8@10251c200
debug: alloc - success - len: 424, ptr_align: 3
info(gpa): small free 224 bytes at u8@102530100
debug: free - len: 224
Bus error at address 0x16db30000
Panicked during a panic. Aborting.
[1]    67842 abort      ./zcached

#

buss error I believe or often missaligned read of memory right ?

spice merlin Jan 19, 2024, 10:36 AM

#

Can you show me how you enabled this debug info for allocations?

#

I dont really know what bus error means

ornate whale Jan 19, 2024, 10:37 AM

#

pub fn main() void {
    var gpa = std.heap.GeneralPurposeAllocator(.{ .verbose_log = true, .retain_metadata = true }){};
    const parent = gpa.allocator();
    var logging_allocator = std.heap.loggingAllocator(parent);
    const allocator = logging_allocator.allocator();

#

this is what I've changed in your main

spice merlin Jan 19, 2024, 10:38 AM

#

Can i keep it in release mode? Teoretically i can based on that what's written in doc you send before

spice merlin Jan 19, 2024, 10:38 AM

#

ornate whale ``` pub fn main() void { var gpa = std.heap.GeneralPurposeAllocator(.{ .verb...

Ty

#

Strange things happend

#

#

I don't really know what happened

ornate whale Jan 19, 2024, 10:45 AM

#

it works for you ?

#

I get crashes on my end ? (btw I compile with the 0.11.0)

spice merlin Jan 19, 2024, 10:46 AM

#

I also use 0.11.0

#

yeah it works now

spice fable Jan 19, 2024, 11:04 AM

#

ally.dupe(u8, key)

#

But yes, your diagnosis sounds entirely correct to me

ornate whale Jan 19, 2024, 11:12 AM

#

Ok good news I've ported your code to 0.12.0 and on my machine it doesn't crash anymore when using the test commands

#

instead I know get this :

~/temp/zcached/zig-out/bin (master*) » ./zcached                             plgol.perso@Pierres-MBP
info(gpa): small alloc 20 bytes at u8@12e00c000
debug: alloc - success - len: 20, ptr_align: 0
info(gpa): small alloc 64 bytes at u8@12e80c000
debug: alloc - success - len: 64, ptr_align: 3
info(gpa): small alloc 26 bytes at u8@12e00c020
debug: alloc - success - len: 26, ptr_align: 3
info(gpa): small free 64 bytes at u8@12e80c000
debug: free - len: 64
info(gpa): small free 20 bytes at u8@12e00c000
debug: free - len: 20
INFO [2024-01-19T12:11:38.000 CET] logger initialized, log_path: ./log/zcached.log
INFO [2024-01-19T12:11:38.000 CET] loading config file from: ./zcached.conf

#

info(gpa): small alloc 144 bytes at u8@12e81c000
debug: alloc - success - len: 144, ptr_align: 3
info(gpa): small alloc 224 bytes at u8@12e81c100
debug: alloc - success - len: 224, ptr_align: 3
info(gpa): small alloc 424 bytes at u8@12e01c200
debug: alloc - success - len: 424, ptr_align: 3
info(gpa): small free 224 bytes at u8@12e81c100
debug: free - len: 224
info(gpa): small alloc 8 bytes at u8@11e00c000
debug: alloc - success - len: 8, ptr_align: 0
info(gpa): small alloc 61 bytes at u8@12e024040
debug: alloc - success - len: 61, ptr_align: 3
error: expand - failure - 61 to 120, buf_align: 3
info(gpa): small alloc 271 bytes at u8@12e01c400
debug: alloc - success - len: 271, ptr_align: 3
error: expand - failure - 8 to 20, buf_align: 0
info(gpa): small alloc 20 bytes at u8@12e00c040
debug: alloc - success - len: 20, ptr_align: 0
info(gpa): small free 8 bytes at u8@11e00c000
debug: free - len: 8
error: expand - failure - 20 to 38, buf_align: 0
info(gpa): small alloc 38 bytes at u8@12e024080
debug: alloc - success - len: 38, ptr_align: 0
info(gpa): small free 20 bytes at u8@12e00c040
debug: free - len: 20
info(gpa): small alloc 38 bytes at u8@12e0240c0
debug: alloc - success - len: 38, ptr_align: 0
INFO [2024-01-19T12:11:43.000 CET] > request: *2\r\n$3\r\nGET\r\n$9\r\nmycounter\r\n
info(gpa): small free 38 bytes at u8@12e0240c0
debug: free - len: 38
DEBUG [2024-01-19T12:11:43.000 CET] handling error: error.NotFound
failed to process command: { 42, 50, 13, 10, 36, 51, 13, 10, 71, 69, 84, 13, 10, 36, 57, 13, 10, 109, 121, 99, 111, 117, 110, 116, 101, 114, 13, 10 }
info(gpa): small free 424 bytes at u8@12e01c200
debug: free - len: 424
info(gpa): small free 271 bytes at u8@12e01c400
debug: free - len: 271
info(gpa): small free 61 bytes at u8@12e024040
debug: free - len: 61
info(gpa): small free 38 bytes at u8@12e024080
debug: free - len: 38
info(gpa): small free 144 bytes at u8@12e81c000
debug: free - len: 144

#

I've also replaced the backing allocator of the gpa by c_allocator instead of page_allocator don't know if it did contribute

#

In case you want to try it for yourself :

pub fn main() void {
    var gpa = std.heap.GeneralPurposeAllocator(.{ .verbose_log = true, .retain_metadata = true }){
         .backing_allocator = std.heap.c_allocator 
     };
    const parent = gpa.allocator();
    var logging_allocator = std.heap.loggingAllocator(parent);
    const allocator = logging_allocator.allocator();

    const result = cli.Parser.parse(allocator) catch {

#

By the way I have to say your code is super easy to read so good job 🙂

spice merlin Jan 19, 2024, 11:17 AM

#

ornate whale Ok good news I've ported your code to 0.12.0 and on my machine it doesn't crash ...

is 0.12 stable yet?

#

Thank y'all guys helping me!

spice merlin Jan 19, 2024, 11:18 AM

#

ornate whale instead I know get this : ``` ~/temp/zcached/zig-out/bin (master*) » ./zcached ...

Can you try run tests?

#

zig test --main-pkg-path .. tests.zig -lc

ornate whale Jan 19, 2024, 11:23 AM

#

The code is not I had to change a few thing here and there namely how you handle file / dir

#

now in 0.12.0 there is no openIterableDir, there is just openDir(path, .{.iterable = true});

spice merlin Jan 19, 2024, 11:24 AM

#

ohh

#

I should port to new version

ornate whale Jan 19, 2024, 11:24 AM

#

same for file so I had to change a few things in persistance.zig

spice merlin Jan 19, 2024, 11:24 AM

#

Is it stable yet right?

ornate whale Jan 19, 2024, 11:24 AM

#

error: unrecognized parameter: '--main-pkg-path'

#

the command doesn't work now I think there was a change to it so it might be it

spice merlin Jan 19, 2024, 11:25 AM

#

Then they changed few things

ornate whale Jan 19, 2024, 11:25 AM

#

but otherwise launching it and doing a bunch of request seems to work just fine

#

do you want me to zip your code ?

spice merlin Jan 19, 2024, 11:26 AM

#

And also seems that openIterableDir was wrapped

#

becouse on every start persister.zig checks persist dir fo latest file

#

No, I'm okay

#

Due to that problem was resolved and SAVE commands also works i need a port to 0.12

ornate whale Jan 19, 2024, 11:28 AM

#

📎 zc12.tar.gz

spice merlin Jan 19, 2024, 11:32 AM

#

Thank you!

ornate whale Jan 19, 2024, 11:33 AM

#

good luck for your project, looks really cool

spice merlin Jan 19, 2024, 11:00 PM

#

spice fable `ally.dupe(u8, key)`

How about unions? can i copy union?

#

I got segmentation fault during saving i think it could be related to same reason

#

error: expand - failure - 94 to 130, buf_align: 3

#

i tried with dup but just not compiles

spice merlin Jan 20, 2024, 7:22 AM

#

And second thing, do i have to free HashMap keys?

spice fable Jan 20, 2024, 10:40 AM

#

spice merlin How about unions? can i copy union?

Dupe will take a slice allocate a new one of the same size and then copy the elements across.
It therefore doesn't work on values like a union.
For that you would need to look at the union value understand if anything in the current active field is allocated and duplicate that if required.

spice fable Jan 20, 2024, 10:42 AM

#

spice merlin And second thing, do i have to free HashMap keys?

The hashmap does not manage the lifetime of its keys it's up to you to ensure that the lifetime is long enough such that the hash map works when you need it to
So hashmap keys being strings doesn't mean implicitly that you have to free it; it depends on the string just like it normally does.

spice merlin Jan 20, 2024, 10:50 AM

#

spice fable Dupe will take a slice allocate a new one of the same size and then copy the ele...

So maybe the better approach is to not use ArenaAllocator, just use standard allocator and only free data on error

#

That way I won't have to copy the values I want to store

spice fable Jan 20, 2024, 10:51 AM

#

spice merlin That way I won't have to copy the values I want to store

Copying the values that you want to store is perfectly fine when using an arena that's actually somewhat expected; because the point of an arena is to make it easy to free everything at once rather than having to go through each individual tiny thing

#

If in fact you find yourself keeping almost everything then maybe that makes more sense but even then it depends

spice merlin Jan 20, 2024, 10:53 AM

#

I'm not sure I should copy because this union might be nested and it might slow the things down

#

pub const AnyType = union(enum) {
    str: []u8,
    sstr: []u8, // simple string
    int: i64,
    float: f64,
    map: map,
    bool: bool,
    array: array,
    null: void,
    // ClientError only for compatibility with ProtocolHandler
    // and it will not be stored in MemoryStorage but will be returned
    err: ClientError,

    pub const array = std.ArrayList(AnyType);
    pub const map = std.StringHashMap(AnyType);
};

pub const ClientError = struct {
    message: []const u8,
};

spice fable Jan 20, 2024, 10:54 AM

#

Structures like that I think tend to lend themselves to being inside arenas because nobody wants to iterate through it recursively and deal with that

#

But sometimes you do with tree like structures like this want to have a function that will recursively duplicate the things.
You're right to consider it a question though because obviously you don't really want to be iterating through a tree more than you have to

#

Sitting here now I might imagine that if you wanted to do this more efficiently you could have some sort of node that has a list of indices where those indices are into a array of strings and then the string variance simply have those indices instead.
The point of this is the array part at the beginning.
This would mean that you could only have to look at the root node for example I trick over that arraylist or slice or whatever of string indices rather than having to walk the entire tree to discover what strings need to be dealt with

spice merlin Jan 20, 2024, 10:59 AM

#

So I should have another ArenaAllocator in my MemoryStorage structure? but I already have TracingAllocator to trace how many bytes were allocated, how can I combine these two allocators?

https://github.com/sectasy0/zcached/blob/master/src/server/storage.zig

GitHub

zcached/src/server/storage.zig at master · sectasy0/zcached

Lightweight and efficient in-memory caching system akin to databases like Redis. - sectasy0/zcached

spice fable Jan 20, 2024, 10:59 AM

#

But of course this involves duplicating the information as to what strings are further down the tree in at least one of the nodes - in this illustration the root note at the top but it depends on exactly what you're trying to do

spice fable Jan 20, 2024, 11:00 AM

#

spice merlin So I should have another `ArenaAllocator` in my `MemoryStorage` structure? but I...

If you were going to go that route then you could put one of the allocators in the structure dynamically allocate the other one and then have 1 point to the other one basically
You could in that particular case though just copy and paste the arena allocator implementation and add to it the ability to trace how many bytes were allocated

#

I feel like just copy and pasting the implementation and adding to it that fairly small feature is probably the better call there

spice merlin Jan 20, 2024, 2:24 PM

#

I wrote copy function at seems that everything works without segfaults. I don't have to copy int's float's and booleans, right?

pub fn copy(value: AnyType, arena: *std.heap.ArenaAllocator) anyerror!AnyType {
    var allocator = arena.allocator();
    switch (value) {
        .str => return  .{ .str = try allocator.dupe(u8, value.str) },
        .sstr => return  .{ .sstr = try allocator.dupe(u8, value.sstr) },
        .int => return .{ .int = value.int  },
        .float => return .{ .float = value.float },
        .bool => return .{ .bool = value.bool },
        .null => return .{ .null = void{} },
        .array => {
            var result = std.ArrayList(AnyType).init(allocator);
            try result.append(try copy(value, arena));
            return .{ .array = result };
        },
        .map => {
            var result = std.StringHashMap(AnyType).init(allocator);

            var iter = value.map.iterator();
            while(iter.next()) |item| {
                const zkey = try copy(.{ .str = @constCast(item.key_ptr.*) }, arena);
                const zvalue = try copy(item.value_ptr.*, arena);

                try result.put(zkey.str, zvalue);
            }

            return .{ .map = result };
        },
        else => return error.UnsuportedType
    }
}

#

Now my 7 tests fails due to that those values are not longer in the same place in memory

#

Can i check somehow that unions are equal? or just expect values inside that unions to be equal?

#

For now i'm not using custom arena tracing allocator

spice fable Jan 20, 2024, 4:57 PM

#

spice merlin Can i check somehow that unions are equal? or just expect values inside that uni...

To my knowledge you cannot test that two unions are equal, because what "equal" means depends on your usecase.

spice merlin Jan 20, 2024, 5:35 PM

#

spice fable To my knowledge you cannot test that two unions are equal, because what "equal" ...

I see, I have to handle it yourself, i just need compare values

#

How about my previous question, I don't need to copy bools floats and ints, right?

#

Becouse dupe gives me strange error when I use it on bools

spice fable Jan 20, 2024, 5:36 PM

#

spice merlin How about my previous question, I don't need to copy bools floats and ints, righ...

bools and ints are not dynamically allocated.

#

They are values that represent a bitpattern of bytes in memory, so having one is the same as having its content

spice merlin Jan 20, 2024, 5:36 PM

#

Now i understand

#

They got fixed size in memory

spice fable Jan 20, 2024, 5:36 PM

#

This property is not the case with things like slices

#

Slices are also a bitpattern in memory -- don't get me wrong.
They consist of 16 bytes; 8 for a pointer, 8 for a count.
But they do NOT contain their own content.

#

Because they point to their 'content'

#

In the same way that a normal pointer does

#

A pointer is an integer; an address.

#

It doesn't contain what it points to

#Data being trash after putting into `std.StringHashMap`