libgpiod icon indicating copy to clipboard operation
libgpiod copied to clipboard

treewide: consider swapping malloc+memset with calloc

Open vfazio opened this issue 2 months ago • 4 comments

There are a number of places where malloc+memset are used to initialize a chunk of memory that could be replaced with a call to calloc. Since we're not allocating large chunks of memory, there may not be huge performance wins from the swap, but it should condense the code a bit.

I acknowledge SF isn't gospel, but here's a quick blurb: https://stackoverflow.com/a/2688522

vfazio avatar Oct 09 '25 14:10 vfazio

I mean, calloc() ultimately does the same thing under the hood, no? I prefer to use it where it's clear we're allocating an array of objects.

brgl avatar Oct 10 '25 08:10 brgl

Ultimately, yes, but I think the libc implementation of calloc can have some optimizations where it can assume that there is no need to clear based on an internal caching/block tracking mechanism.

At least on my system, calloc seems to be slightly faster, but that may be down to memory usage patterns:

vfazio@vfazio4:~/development/libgpiod$ hyperfine -N -w 10 --runs 15000 ./malloc ./calloc
Benchmark 1: ./malloc
  Time (mean ± σ):     341.9 µs ±  50.4 µs    [User: 220.5 µs, System: 81.8 µs]
  Range (min … max):   293.1 µs … 1083.4 µs    15000 runs
 
  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
 
Benchmark 2: ./calloc
  Time (mean ± σ):     337.6 µs ±  48.1 µs    [User: 217.4 µs, System: 80.9 µs]
  Range (min … max):   285.5 µs … 1350.7 µs    15000 runs
 
  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.
 
Summary
  ./calloc ran
    1.01 ± 0.21 times faster than ./malloc
vfazio@vfazio4:~/development/libgpiod$ cat calloc.c
#include <stdio.h>
#include <stdlib.h>
#include "gpiod.h"
#include "internal.h"

struct gpiod_line_info {
	unsigned int offset;
	char name[GPIO_MAX_NAME_SIZE + 1];
	bool used;
	char consumer[GPIO_MAX_NAME_SIZE + 1];
	enum gpiod_line_direction direction;
	bool active_low;
	enum gpiod_line_bias bias;
	enum gpiod_line_drive drive;
	enum gpiod_line_edge edge;
	enum gpiod_line_clock event_clock;
	bool debounced;
	unsigned long debounce_period_us;
};

int main()
{
	struct gpiod_line_info *info;
	for (int i = 0; i <100; i++)
	{
		info = (struct gpiod_line_info*)calloc(1,sizeof(*info));
		info->offset = 1;
		info->used = false;
		free(info);
	}
}

vfazio avatar Oct 10 '25 11:10 vfazio

I mean... Sure, send the patches, I won't reject them. But it's not like it's a really hot path here, is it?

brgl avatar Oct 10 '25 13:10 brgl

I mean... Sure, send the patches, I won't reject them. But it's not like it's a really hot path here, is it?

Correct, it's not a hot path. I was thinking of this as more of a starter issue if someone wants to help with contributing to the project since the barrier to entry is pretty low and there is no real urgency.

vfazio avatar Oct 10 '25 13:10 vfazio