Re: [PATCH v5 7/7] fsmonitor: add a performance test

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 14, 2017, 18:36 UTC
Message-ID: <xmqqvany2z84.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <9c1ed8d4-6bdb-e709-758d-4b010525e9e3@gmail.com>

Ben Peart <peartben@gmail.com> writes:

Show 8 quoted lines

>> Having said all that, I think you are using this ONLY on windows;
>> perhaps it is better to drop #ifdef GIT_WINDOWS_NATIVE from all of
>> the above and arrange Makefile to build test-drop-cache only on that
>> platform, or something?
>
> I didn't find any other examples of Windows only tools.  I'll update
> the #ifdef to properly dump the file system cache on Linux as well and
> only error out on other platforms.

If this will become Windows-only, then I have no problem with platform specfic typedef ;-) I have no problem with CamelCase, either, as that follows the local convention on the platform (similar to those in compat/* that are only for Windows).

Having said all that.

Another approach is to build this helper on all platforms, with sections protected by "#ifdef LINUX", "#ifdef WINDOWS", etc. That way, the platform detection and switching between running this program and echoing something into /proc filesystem performed in p7519 can be removed (this test-helper program will be responsible to implement that logic instead). When p7519 wants to drop the filesystem cache, regardless of the platform, it can call this test-helper without having to know how the filesystem cache is dropped.

I do not have strong preference either way, but I have a slight suspicion that the "another approach" above may give us a better result. For one thing, the test-helper can be reused in new perf scripts people will write in the future.

Thanks.