456603 - get rid of Windows stat cache

Status: REOPENED

(Core :: XPCOM, defect)

Description

[Josh Aas]

This is the Windows part of bug 307815, we don't have to remove stat caches for all platforms at the same time. Our current stat caches are fundamentally flawed, we should remove them.

Comment 1

[Josh Aas]

Attachment: fix v1.0

Basic try server run shows no significant perf impact, we'll see better numbers when this lands on trunk.

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 340064 [details] [diff] [review]
fix v1.0

drop the braces for one liners.

     if (aFileSize != -1 && SetEndOfFile(hFile))
     {
-        MakeDirty();
         rv = NS_OK;
     }




assuming that this does not cause any performance regression as mentioned in bug 307815, this is fine.

Comment 3

[Josh Aas]

landed on trunk

Comment 4

[Josh Aas]

backed out because of tbox issues, not necessarily anything wrong with the patch

Comment 5

[Dave Townsend [:mossop]]

This might have caused a performance regression on Tp on Vista (some 30% increase). It is confusing though since the first talos runs also supposedly included the backout, but Tp consistently went up on all 4 Vista talos machines then back down again one cycle later. This was the only patch liable to have impacted, but not sure what is going on. Might be good to land in a quiet tree next time.

Comment 6

[Josh Aas]

Attachment: windows errors

this is a text file containing the unit test errors that came up

Comment 7

[Ben Campbell]

I think this is worth revisiting. I can't imagine that stat() is performance-critical these days, even on Windows.
Is there a good reason not to ditch the stat cache?
If so, there should at least be a note added to nsIFile!

If anyone has any strong opinion either way, NI me if you'd like me to work up a patch.

(It just bit me in Thunderbird - Bug 1900172).

Comment 8

[Dave Townsend [:mossop]]

Wanting to bring data to this I threw a quick and dirty patch that bypassed the stat cache and ran it through our performance tests. Tests on a single revision are not super reliable but nothing jumped out as a regression so this seems like something we could do.

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=a07238f2e1ac5ac7e45425563442b8164e4d08a4&newProject=try&newRevision=621fea9c0d82ba54813466842923443e939d7711&page=1