Hello Dmitry,

  you are right about the strign allocation, we have this in 4 and 5 as
well. But a minor change would even captutre that too:

typedef struct bucket {
        ulong h;        /* Used for numeric indexing */
        uint nKeyLength;
        void *pData;
        void *pDataPtr;
        struct bucket *pListNext;
        struct bucket *pListLast;
        struct bucket *pNext;
        struct bucket *pLast;
        HashKey key;
        union {
                char  s[];   /* Must be last element */
                UChar u[];   /* Must be last element */
        } arKey;
}

Also you had that member being [1] though the language allows to have it be
an incomplete type ([]). I prefer the latter becuase that does not force to
allocate a not alignable memory block in the integer index case. If we
preceed this way there is no need to allocate two blocks and instead we can
easily fill in the pointer for the string key by a simple addition:

// for the integer index case
bucket p = pemalloc(sizeof(bucket), persistent);

// for the string index case
bucket p = pemalloc(sizeof(bucket)+key_len, persistent);
p->key.arKey.s = p + offsetof(p, arKey);
memmove(p->key.s, key_value, key_len);

My opinion here is anyway that having to deal with only one struct is much
easier and also faster. Being able to change the malloc to a single pointer
add operation is a nice addon.

best regards
marcus

Sunday, March 12, 2006, 11:11:32 AM, you wrote:

> Hi Marcus,

> The idea make sense, but note that this modification will need two malloc()
> for each bucket with string index.
> First for bucket itself and the second for "arKey". (now they are done in
> one call)
> Also we will lose some performance in dereferencing pointer to arKey in each
> ZendHash operation.

> Thanks. Dmitry.


>> -----Original Message-----
>> From: Marcus Boerger [mailto:[EMAIL PROTECTED] 
>> Sent: Saturday, March 11, 2006 3:22 PM
>> To: php-i18n@lists.php.net
>> Subject: [PHP-I18N] Hash api change
>> 
>> 
>> Hello php-i18n,
>> 
>>   i think we should change the api a bit:
>> 
>> From:
>> 
>> typedef struct _key {
>>         zend_uchar type;
>>         union {
>>                 char  s[1];   /* Must be last element */
>>                 UChar u[1];   /* Must be last element */
>>         } arKey;
>> } HashKey;
>> 
>> typedef struct bucket {
>>         ulong h;                                              
>>   /* Used for numeric indexing */
>>         uint nKeyLength;
>>         void *pData;
>>         void *pDataPtr;
>>         struct bucket *pListNext;
>>         struct bucket *pListLast;
>>         struct bucket *pNext;
>>         struct bucket *pLast;
>>         HashKey key; /* Must be last element */
>> } Bucket;
>> 
>> To:
>> 
>> typedef struct _key {
>>         ulong h;     /* Used for numeric indexing */
>>         uint nKeyLength;
>>         zend_uchar type;
>>         zstr arKey;   /* Must be last element */
>> } HashKey;
>> 
>> typedef struct bucket {
>>         void *pData;
>>         void *pDataPtr;
>>         struct bucket *pListNext;
>>         struct bucket *pListLast;
>>         struct bucket *pNext;
>>         struct bucket *pLast;
>>         HashKey key; /* Must be last element */
>> } Bucket;
>> 
>> So now HashKey matches zend_hash_key just by pure reordering.
>> 
>> Also we should probably change the apply_with_arguments stuff 
>> to pass the tsrm key. That is we'd change from:
>> 
>> typedef int (*apply_func_args_t)(void *pDest, int num_args, 
>> va_list args, zend_hash_key *hash_key); ZEND_API void 
>> zend_hash_apply_with_arguments(HashTable *ht, 
>> apply_func_args_t apply_func, int, ...);
>> 
>> 
>> To:
>> 
>> typedef int (*apply_func_args_t)(void *pDest TSRMLS_DC, int 
>> num_args, va_list args, zend_hash_key *hash_key); ZEND_API 
>> void zend_hash_apply_with_arguments(HashTable *ht TSRMLS_DC, 
>> apply_func_args_t apply_func, int, ...);
>> 
>> Further more our add_assoc functions currently only allow 
>> handling of native string indexes. This should be changed to 
>> allow unicode indexes as well:
>> 
>> ZEND_API int add_assoc_long_ex(zval *arg, char *key, uint 
>> key_len, long n);
>> 
>> TO:
>> 
>> ZEND_API int add_assoc_long_ex(zval *arg, zend_uchar type, 
>> zst *key, uint key_len, long n);
>> 
>> Though maybe it is considerable to just add more functions i 
>> don't think flooding the api is a good idea and prefer to 
>> only have one version that can easily deal with both.
>> 
>> -- 
>> Best regards,
>>  Marcus                          mailto:[EMAIL PROTECTED]
>> 
>> -- 
>> PHP Unicode & I18N Mailing List (http://www.php.net/)
>> To unsubscribe, visit: http://www.php.net/unsub.php
>> 
>> 
>> 




-- 
Best regards,
 marcus

-- 
PHP Unicode & I18N Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to