View Issue Details

IDProjectCategoryView StatusLast Update
0000615LDMud 3.3Implementationpublic2009-05-10 07:39
Reporterzesstra Assigned Tozesstra  
PrioritynormalSeverityminorReproducibilityN/A
Status resolvedResolutionfixed 
Product Version3.3.718 
Target Version3.3.719Fixed in Version3.3.719 
Summary0000615: RfC: Replace SINT in slaballoc.c/smalloc.c by sizeof(word_t)
DescriptionDuring tracing 0000611 I noticed that in slaballoc.c and smalloc.c the define SINT (SIZEOF_CHAR_P) is heavily used as the size of a word. I don't know the origin of SINT, but I think the semantic of it is not very apparant and there isn't even a comment.

a) One suggestion is to replace it by a sizeof(word_t), because it is used as such. sizeof(word_t) is the same as SIZEOF_CHAR_P [0], because word_t is typedef'ed to p_uint and p_(u)int are integers with the same size as a pointer.
b) Another possibility is, to give SINT a new name which directly reflects meaning as sizeof(word_t). But I don't think, SIZOF_WORD_T is any better than sizeof(word_t).
c) And the third one: replace calculations with char* (e.g. block + 8*sizeof(word_t)) by the corresponding ones with word_t*, e.g. (word_t)block + 8, which is the most effort, but IMHO the cleanest approach. I would see it more as independent measure to a) or b).

[0]: I am not sure, but I think the standard allows pointers to have different sizes, e.g. sizeof(void*)!=sizeof(int*). On systems like that we may be in rather bad shape...

Do you have any opinions about this?
TagsNo tags attached.

Activities

zesstra

2009-03-13 16:52

administrator   ~0000996

BTW:
the comment about CHUNK_SIZE says: "make sure that the resulting chunk sizes are SINT aligned."
We may discuss replacing
#define MEM_ALIGN (SIZEOF_CHAR_P)
by
#define MEM_ALIGN (sizeof(word_t))
as well.

fufu

2009-03-13 18:14

manager   ~0000998

I don't like the idea of replacing a dedicated #define, however ill-named, by an anonymous sizeof().

How about renaming SINT to GRAN for "granularity"? We can then define MEM_ALIGN as GRAN.

I have no opinion on whether sizeof(word_t) is better than SIZEOF_CHAR_P.

slaballoc.c uses the same definitions, so whatever changes we decide on should be made in both smalloc.c and slaballoc.c.

zesstra

2009-03-13 19:02

administrator   ~0001000

Usually I would agree. But in this case I am not sure. Both allocators base on word_t:
* Allocations are measured in 'word_t's which are the same size as void*.

Which means, that you anyway can't change SINT/GRAN/whatever from being sizeof(word_t), because first you would have to decouple the rest of the file from word_t - which is a lot. And I would not see any reason to do so at the moment, because word_t is already an abstraction which can be changed.
We might decide not to base the allocations on void*/p_uint and increase/decrease granularity. But in any case we would IMHO just change word_t. (Ok, maybe something breaks in that case, because some code may rely on word_t being the size of void*/p_uint.)

zesstra

2009-04-16 15:07

administrator   ~0001050

Ok... The state is now, that we can't change any granularity and word_t independently.
Maybe de-coupling the two things is a nice idea, but I won't do now, because it is a lot of work and IMHO not that important (any other volounteers?).

I can use a better-named enum/define, but until someone checks the complete allocator, that will be IMHO very incomplete (and at some points may be even mis-used). Therefore I am not sure, if this does not lead to confusion (the false impression that we could change a granularity independently of word_t and that all uses of GRAN are correct). A comment may alleviate that somewhat. But for a long time (if not forever), any GRAN has to be sizeof(word_t).

zesstra

2009-05-10 07:39

administrator   ~0001089

Ok, I want this from my to-do list.
I replaced SINT by GRANULARITY and defined MEM_ALIGN to be GRANULARITY in r2575.
(BTW: I had to replace an #if by if, but as the expression is constant, the compiler will remove the if anyway.)

Issue History

Date Modified Username Field Change
2009-03-13 16:45 zesstra New Issue
2009-03-13 16:45 zesstra Status new => assigned
2009-03-13 16:45 zesstra Assigned To => zesstra
2009-03-13 16:52 zesstra Note Added: 0000996
2009-03-13 18:14 fufu Note Added: 0000998
2009-03-13 19:02 zesstra Note Added: 0001000
2009-04-16 15:07 zesstra Note Added: 0001050
2009-05-10 07:39 zesstra Note Added: 0001089
2009-05-10 07:39 zesstra Status assigned => resolved
2009-05-10 07:39 zesstra Fixed in Version => 3.3.719
2009-05-10 07:39 zesstra Resolution open => fixed