git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

From
Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date
Jul 16, 2013, 21:36 UTC
Message-ID
<51E5BCDF.5070004@ramsay1.demon.co.uk>
In-Reply-To
<51E2CE97.2040900@gmail.com>
Mark Levedahl wrote:
Show 22 quoted lines
> On 07/10/2013 04:23 PM, Ramsay Jones wrote:
>> Commit adbc0b6b ("cygwin: Use native Win32 API for stat", 30-09-2008)
>> added a Win32 specific implementation of the stat functions. In order
>> to handle absolute paths, cygwin mount points and symbolic links, this
>> implementation may fall back on the standard cygwin l/stat() functions.
>> Also, the choice of cygwin or Win32 functions is made lazily (by the
>> first call(s) to l/stat) based on the state of some config variables.
>>
>> Unfortunately, this "schizophrenic stat" implementation has been the
>> source of many problems ever since. For example, see commits 7faee6b8,
>> 79748439, 452993c2, 085479e7, b8a97333, 924aaf3e, 05bab3ea and 0117c2f0.
>>
>> In order to limit the adverse effects caused by this implementation,
>> we provide a new "fast stat" interface, which allows us to use this
>> only for interactions with the index (i.e. the cached stat data).
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>> ---
> 
> I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results 
> using your prior patch (removing the Cygwin specific lstat entirely) and 
> get the same results with both, so this seems ok from me.
Thank you for testing this.
Show 12 quoted lines
> My comparison point was created by reverting your current patch from pu, 
> then reapplying your earlier patch on top, so the only difference was 
> which approach was used to address the stat functions.
> 
> Caveats:
> 1) I don't find any speed improvement of the current patch over the 
> previous one (the tests actually ran faster with the earlier patch, 
> though the difference was less than 1%).
> 2) I still question this whole approach, especially having this 
> non-POSIX compliant mode be the default. Running in this mode breaks 
> interoperability with Linux, but providing a Linux environment is the 
> *primary* goal of Cygwin.

Yes, I agree. Since I _always_ disable the Win32 stat functions (by setting core.filemode by hand - yes it's a little annoying), I don't get any "benefit" from the added complexity.

However, I don't use git on cygwin with *large* repositories. git.git is pretty much as large as it gets. So, the difference in performance for me amounts to something like 0.120s -> 0.260s, which I can barely notice.

Other people may not be so lucky ...

I would be happy if my original patch (removing the win32 stat funcs) was applied, but others may not be. :-P

ATB, Ramsay Jones

Previous: Mark LevedahlNext: Mark Levedahl
Message 14 of 16 in “cygwin: Add fast_lstat() and fast_fstat() functions”
  1. 1/1 cygwin: Add fast_lstat() and fast_fstat() functionsRamsay Jones, Jul 10, 2013
  2. Mark LevedahlJul 14, 2013
  3. Junio C HamanoJul 15, 2013
  4. Torsten BögershausenJul 16, 2013
  5. Mark LevedahlJul 16, 2013
  6. Dmitry PotapovJul 16, 2013
  7. Mark LevedahlJul 16, 2013
  8. Ramsay JonesJul 18, 2013
  9. Torsten BögershausenJul 18, 2013
  10. Mark LevedahlJul 18, 2013
  11. Junio C HamanoJul 18, 2013
  12. Mark LevedahlJul 19, 2013
  13. Mark LevedahlJul 16, 2013
  14. Ramsay JonesJul 16, 2013
  15. Mark LevedahlJul 16, 2013
  16. Junio C HamanoJul 18, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.