Page MenuHomePhabricator

Don't use mmap on Windows
AbandonedPublic

Authored by yvvan on Jul 10 2017, 7:36 AM.

Details

Summary

Memory mapping does not make llvm faster (at least I don't see any difference).
It also allows llvm not to lock files which is sometimes quite important.

Diff Detail

Event Timeline

yvvan created this revision.Jul 10 2017, 7:36 AM
yvvan added a comment.Jul 12 2017, 2:36 AM

What do you think about that change?

nik added a subscriber: nik.

Ping.

I guess locking the file less helps also clangd?!

ilya-biryukov edited edge metadata.Aug 17 2017, 1:57 AM

This looks strange. AFAIK, memory mapping files in Windows does no 'locking' by itself. I've just written a small program to check that it's possible to modify the file, memory-mapped using Win32 API.
What is specific problem you ran into?

yvvan added a comment.Aug 17 2017, 2:04 AM

The files might also be removed.
I checked that with qtcreator from git. Switching to the different branch started reparse of some files.
When I frequently switched branches back and forth I got lock issues. With mmap turned off this did not happen.

ilya-biryukov added a comment.EditedAug 17 2017, 2:37 AM

The files might also be removed.
I checked that with qtcreator from git. Switching to the different branch started reparse of some files.
When I frequently switched branches back and forth I got lock issues. With mmap turned off this did not happen.

Renaming/removing memory mapped files also works fine.

What were those 'lock issues' specifically? MemoryBuffer code that uses the function you're changing seems to be doing the right thing by always allowing shared file access.
We definitely need a test case that realiably reproduces your issue.

joerg added a subscriber: joerg.Aug 17 2017, 4:48 AM

The primary reason for using mmap is not so much performance, but reduced memory foot print.

yvvan abandoned this revision.Mar 12 2018, 6:49 AM

It was the wrong direction