Eliminate CGo usage from cgroup package #1238
Conversation
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
| } | ||
| ret := *(*uint64)(unsafe.Pointer(&hf.Bytes()[0])) |
There was a problem hiding this comment.
Writing down some notes, as I was confused at first. Reading the source code helped quite a bit 😄 .
Please feel free to correct this if I got some stuff wrong.
Before
We always allocated cgid_file_handle with uint64_t cgid (the handle data), which should always fit.
Now
.Bytes + .Type are allocated + 32 Bytes for the handle, which should be enough for us.
Then the Bytes() call that was throwing me off because thought that it was returning the whole raw buffer, including Bytes and Type, but they are skipped.
One more thing I wonder why they did .Bytes +4 rather than something like the below which I think would be easier for maintenance and clearer? I guess it's due to perf reasons, but not sure.
return unsafe.Slice((*byte(unsafe.Pointer(uintptr(unsafe.Pointer(&fh.fileHandle.Bytes))+unsafe.Sizeof(&fh))), n)
|
The ID extraction mechanism has issues (current and upcoming). I'm double-checking it. |
|
The other day I forgot to suggest this, should we add this new implementation while keeping the old one and then run both and compare for mismatches? That way we can ensure that the new implementation behaves exactly as the current one. What do you think? |
For sure. FWIW, the previous implementation also gives an empty string. On the other hand, I'm also questioning the need for this. It's not used anywhere. And we don't add it as a metadata label. |
Signed-off-by: Kemal Akkoyun kakkoyun@gmail.com
We should remove as many CGo dependencies as possible. Switching from Go to C code at runtime always comes with increased costs in terms of CPU usage, so that is another reason to avoid mixed code.
TODO