I like these changes -- fix the constant, make it tunable, keep it simple.

86-90:  I think for a big theory statement we should document what the
parameters are for the hash table size, rather than the exact algorithm for
achieving it.  More of the "why" and less of the "how".  You might mention:
 - number of buckets is at least enough to have 1 bucket for every 8k of
physical memory
 - number of buckets is a power of 2
 - at least 4096 buckets
 - results in about 1MB of hash tables for every 1GB of memory.

233: zfs_arc_ht_base_masklen - I don't understand the name of this
variable.  I would recommend:

/*
 * This variable sets the assumed average block size.  The ARC hash table
will be sized large enough to
 * not have too much chaining even if all of memory used to cache blocks of
this size.
 */
uint64_t zfs_arc_average_block_size = 8 * 1024;

(buf_init() will then need to do a multiply rather than <<(ht_maxlen +
this).)

632: declare each member on its own line.

748: does declaring it inline improve performance?  I suspect that it does,
at least on gcc with illumos.  But in the past "inline" has been ignored
given the compiler flags used, so it's worth checking.

983: doesn't ht_num_locks need to be a power of 2?  physmem is not
necessarily a power of 2.

If you want to add a tunable like zfs_arc_average_block_size for the
dbuf_hash_table, that would be cool too (to replace the constant 4096 in
dbuf_init).

--matt



On Wed, Oct 16, 2013 at 4:09 PM, Saso Kiselkov <[email protected]>wrote:

> Okay, I've reworked and simplified the ARC buf hash changes to just
> change the way we size the hash table and bump up the amount of hash
> tables we allocate to 1MB/1GB.
> Webrev: http://cr.illumos.org/~webrev/skiselkov/new_buf_hash/
> Issue: https://www.illumos.org/issues/4218
>
> Cheers,
> --
> Saso
>
>
> -------------------------------------------
> illumos-zfs
> Archives: https://www.listbox.com/member/archive/182191/=now
> RSS Feed:
> https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
> Modify Your Subscription:
> https://www.listbox.com/member/?member_id=21635000&id_secret=21635000-73dc201a
> Powered by Listbox: http://www.listbox.com
>
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer

Reply via email to