Re: [PATCH] Improve random memcpy benchmark

[Paul Clarke <pc at us dot ibm dot com>]

On 2/11/20 12:32 PM, Wilco Dijkstra wrote:
>> -#define MIN_PAGE_SIZE 131072
>> +#define MIN_PAGE_SIZE (512*1024+4096)
> 
>> Is the above expected to be a multiple of the system page size?
>> (Not all systems use 4096 byte pages or factors thereof).
> 
> It's just badly named. It's not even rounded up to a multiple of the
> page size but I suppose mmap doesn't mind too much. The +4096
> above isn't needed since the actual mmap call uses at least double
> the requested size (2x MIN_PAGE_SIZE or 3x getpagesize()), which
> is wrong too. 

At least some of the tests do attempt to align to a page boundary, like in benchtests/bench-strcmp.c:
  /* Put them close to the end of page.  */
  i = align1 + CHARBYTES * (len + 2); 
  s1 = (CHAR *) (buf1 + ((page_size - i) / 16 * 16) + align1);
  i = align2 + CHARBYTES * (len + 2); 
  s2 = (CHAR *) (buf2 + ((page_size - i) / 16 * 16)  + align2);

So modifying MIN_PAGE_SIZE so that it's definitely not a multiple of any page size except 4096 might have some unexpected impact.

If the +4096 isn't needed, why add it?

> It's something which still need to be cleaned up in the benchtests -

Agreed.

> we should just have a macro with the memory size rather than
> messing about with page sizes.

Or, do page size correctly. There are operations which are sensitive to page boundaries that need to be tested.

I understand the current code may not do page size correctly.  It doesn't.  But, I think we should keep the implementation close to the original intent until it is cleaned up properly.

PC