Re: [PATCH] ntdll: Filling in memory counters under OS X.

[Ken Thomases]

Hi,

Thanks for the submission.  See my comments below.

On Jan 27, 2016, at 10:19 AM, Snorri Sturluson <snorri.sturluson(a)ccpgames.com> wrote:
> +#ifdef __APPLE__
> +#include <mach/mach.h>
> +#endif

We're not totally consistent about this, but probably better to conditionalize this (and the code below) on HAVE_MACH_MACH_H instead of __APPLE__.
> +#if defined(__APPLE__)
> +
> +void fill_VM_COUNTERS(VM_COUNTERS* pvmi)

This should be "static" (in both branches of the #if).
> +{
> +    struct mach_task_basic_info info;
> +    mach_msg_type_number_t infoCount = MACH_TASK_BASIC_INFO_COUNT;
> +    if(task_info(mach_task_self(), MACH_TASK_BASIC_INFO, (task_info_t)&info, &infoCount) == KERN_SUCCESS)

You need to conditionalize this on MACH_TASK_BASIC_INFO being defined.  It is not defined in the 10.5 SDK and Wine needs to compile against that.  (Well, Alexandre may have finally given up on 10.5, but it's also not in the 10.6 SDK.)  If it's not defined, you can fall back to TASK_BASIC_INFO_64 / struct task_basic_info_64, although it doesn't have as much information.
> +    {
> +        pvmi->VirtualSize = info.resident_size + info.virtual_size;
> +        pvmi->PagefileUsage = info.virtual_size;
> +        pvmi->WorkingSetSize = info.resident_size;

pvmi->PeakWorkingSetSize can be set from info.resident_size_max, right?  The other two Peak* fields could be set based on the current info.virtual_size (possibly combined with resident_size_max), at least.

You could also track the highest value seen by this code, which is not the same as the real peak but at least never decreases.  You would have to be careful to do it in a thread-safe manner, probably with atomic compare-and-swap in a loop.
> +    }

You can get PageFaultCount from TASK_EVENTS_INFO / struct task_events_info.faults.

Regards,
Ken