[PATCH] D48501: [dsymutil] Introduce a new CachedBinaryHolder

Jonas Devlieghere via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 29 06:37:49 PDT 2018


JDevlieghere added a comment.

In https://reviews.llvm.org/D48501#1145789, @friss wrote:

> Why not transition completely to the new implementation and remove the old code in one step?


I wanted to do that in a separate patch: https://reviews.llvm.org/D48770

> Should we have a limited number of files that we keep open? Some systems might run with a low number of allowed open files per-process.

The files are closed immediately after being read by the MemoryBuffer. While we keep the files in memory the FD is already closed.

> This code is supposed to handle a lot of weird cases, can you make sure that all of this still works:
> 
> - fat object files
> - thin archive containing fat object-files
> - fat archive containing thin objectfiles (I think the current code handles all of them, but I might be wrong. Please double-check)

This logic didn't change, all of them are still supported. (I paid special attention to this while doing the refactoring)


https://reviews.llvm.org/D48501





More information about the llvm-commits mailing list