CODEWITHSUNDEEP
clistoperatorslinked-list
user905292
user905292

Reputation: 21

can this code be optimized?

I think I'm getting obsessed with optimization so I wonder it the following code can be "optimized":

Let's say I have a C language linked list, and when creating a new element I use this code:

log_event_list_cur->next = 
    (struct log_event_list *)malloc(sizeof(struct log_event_list));

log_event_list_cur = log_event_list_cur->next;

I wonder if the following code would be ok:

log_event_list_cur = 
    log_event_list_cur->next = 
        (struct log_event_list *) malloc(sizeof(struct log_event_list));

or:

log_event_list_cur->next = 
    log_event_list_cur=(struct log_event_list *) malloc....

Regards!.

Upvotes: 1

Views: 156

Answers (5)

CAFxX
CAFxX

Reputation: 30341

As already pointed out, the compiler will likely emit the same code for all the three versions.

Instead, if you really want to make it faster, implement a free-list, i.e. a second list that holds the currently unused list items. This way, "allocating" a new member means just popping an item out of the free-list (similarly "freeing" means simply pushing the item on the free-list). This way you don't have the malloc/free overhead for every single new "allocation". Obviously, if the free-list is empty and you need to allocate a new member, you'll have to call malloc anyway, but hopefully this will happen rarely.

BTW, I hope you just omitted the check for the return value of malloc. Otherwise, if malloc returns NULL, you'll likely get a crash soon afterward...

Upvotes: 1

shelleybutterfly
shelleybutterfly

Reputation: 3247

As mentioned in the other answers, the compiler should be able to optimize out any differences such as you show; I would probably go with the first for readability, and also for readability I'd probably either set up a #define (or a const in later versions of C) with the value for your sizeof call, and a typedef for your struct to compress the size a bit.

[edit: as per a comment on the question, the cast isn't even necessary, removed.]

[edit: as per another comment on the question sizeof(log_node) as a const no longer makes any sense at all; it (sort of) did when it was c_log_node_size vs. sizeof(struct log_event_list), but now it's just totally silly. (there are other good reasons to not do it as noted in the comment, maybe sizeof_c_log_node could be ok? no no no.) :D]

typedef struct log_event_list log_node

and then it becomes:

log_event_list_cur->next = malloc(sizeof(log_node));
log_event_list_cur = log_event_list_cur->next;

If you were wanting to do some optimization, on this particular part of your code, first I would suggest that you do some profiling of your system to make sure it's actually something that's a bottleneck. If it's not causing issues, then there's no need for optimizations, any time you spent optimizing would have been better spent elsewhere, since it didn't make a difference anyway. Probably there's something that could use optimization, it just might not be this. But, given this particular block of code the only thing that comes to mind is to optimize the allocation of your blocks.

I started to write some code for that, but I didn't leave it because it occurred to me that there's no need at all to reinvent the wheel on this one, other than that it's interesting. If you find that it would be valuable to optimize that bit of code, here's a possibility for tweaking your allocations: vmalloc. I wouldn't mess with it unless you could really demonstrate that it was a bottleneck however. It's kinda neat to think about though. :)

Upvotes: 0

n. m. could be an AI
n. m. could be an AI

Reputation: 120049

Stop being obsessed with optimization, start being obsessed with readability. Premature optimization is the root of all evil.

The first code fragment are OK. It is clear what's going on. The linked list gets a new entry after the current one (presumably it's the last one), and then the current is moved forward to become the last.

It is much harder to understand what's going on in the second fragment. It is ultimately the same thing as the first example, but one needs to apply mental effort to make sure it really is.

The third fragment is all wrong, and it's the perfect example of why you should not even start to think about starting to think about optimizations before it's time to do so. Make it right, then make it fast, and only do the latter if you have hard cold profiler data before your very own eyes.

Upvotes: 0

Yann Ramin
Yann Ramin

Reputation: 33197

Its not going to make any difference. Simple assignments will be optimized away by the compiler anyway.

Learn at the very least how to get the "gist" of assembly code, and how to dump assembly output from your executable. On Linux, using objdump -S -d will give you inline code with assembler.

Upvotes: 1

Paul
Paul

Reputation: 141887

Yes,the first one is fine (correct code that behaves equivalently to the first code), but neither are optimizations. They will be compiled into the same machine code. Do what you find most readable.

The second one behaves differently since log_event_list_cur gets assigned to a new list entry before the next is set.

Upvotes: 5

Related Questions