Ticket #1741 (closed feature: fixed)

Opened 4 years ago

Last modified 4 years ago

Macro abstraction for hash iterations

Reported by: luben Owned by:
Priority: normal Milestone:
Component: core Version: 2.6.0
Severity: medium Keywords: hash macro
Cc: Language:
Patch status: new Platform: all

Description

A lot of files in the source tree poke with hash.c internals. I have extracted 2 common patterns as C preprocessor macros and have replaced the manual iteration with macro iteration in a lot of PMCs.

Benefits: 1. Less code 2. Encapsulation - if you want to change hash internals you have to edit only hash.c and hash.h

Better names for macros are appreciated

Attachments

Change History

Changed 4 years ago by luben

patch

Changed 4 years ago by luben

Upgated patch

Changed 4 years ago by luben

I have added updated patch with macro for linear hash iteration. It uses some knowledge about our internal hash structure and seems faster.

Changed 4 years ago by chromatic

On Saturday 14 August 2010 at 10:20, Parrot  wrote:

>  I have added updated patch with macro for linear hash iteration. It uses
>  some knowledge about our internal hash structure and seems faster.

This patch seems to work well too.  Any other testers on other platforms?

-- c 

Changed 4 years ago by luben

I am adding a series of patches over the first. The only questionable patch is

0004-get-rid-of-parrot_hash_get_idx-very-ugly-crap.patch

which deletes a function we do not need anymore. I am not sure about our depreciation/support policy and how we should proceed with this change

Changed 4 years ago by luben

Changed 4 years ago by luben

Changed 4 years ago by luben

Adding two patches with fixes:

0008:

Rename INITIAL_BUCKETS to INITIAL_SIZE

INITIAL_BUCKETS name is misleading. This define is used for the size of the index. The number of allocated buckets is N_BUCKETS(INITIAL_SIZE) and could be differend.

0009:

Important fixes

  • make hash again work with N_BUCKETS(X) != X
  • fix memory allocation problems catched by more strict allocators. libhoard catched some use-after-free bugs.
  • realocate pointers in linear scan - it's faster

Changed 4 years ago by luben

Changed 4 years ago by luben

Changed 4 years ago by chromatic

I've applied the first two patches, but the third gives me a segfault in the memmove in expand_hash(). Can you reproduce?

Changed 4 years ago by cotto

0005 was applied as r48686, 0006 was applied as r48687, 0007 was applied by someone else and 0008 was applied as r48688.

Thanks for the patches. The hash code needs some love and I'm glad you've taken the effort to help clean it up. In the future, please keep only a few patches to a ticket. More concise filenames would also be nice.

Changed 4 years ago by cotto

0004 also appears to have been applied. Only functions marked PARROT_API need a deprecation cycle for removal, so it's clear. That leaves 0003 and 0009 to be committed. I tried both but ran into some conflicts. Could you provide updated versions against trunk?

Changed 4 years ago by cotto

  • status changed from new to closed
  • resolution set to fixed

luben says that all the important code is in trunk, so I'm marking this as resolved.

Note: See TracTickets for help on using tickets.