| Age | Commit message (Collapse) | Author |
|---|
|
In absence of a better name, this commit refactors the shared cache code.
|
|
Also fixes a BinaryViewRef leak
|
|
The initial state is initialized during `PerformInitialLoad` and is
immutable after that point. This required some slight restructuring of
how information about memory regions is tracked as that was previously
modified as regions were loaded. Memory regions are now stored in a map
from their address range to the `MemoryRegion` object. This makes it
cheap to look them up by address which is a common operation.
The modified state consists of changes since the last save to the
`DSCView` / `ViewSpecificState`. This means it is no longer necessary to
copy any state when mutating a `SharedCache` instance for the first
time. Instead, its data structures start off empty and are populated as
images, sections, or symbol information is loaded.
The loaded state consists of all modified state that has since been
saved. It lives on the `ViewSpecificState`. Saving modified state
merges it into the the existing loaded state.
This pattern is carried over to the `Metadata` stored on the `DSCView`.
The initial state is stored under its own metadata key, and each
modified state is stored under a key with an incrementing number. This
means each save of the state only needs to serialize the state that
changed, rather than reserializing all of the state all of the time.
There are two huge benefits from these changes:
1. At no point does `SharedCache` have to copy its in memory state.
The basic copy-on-write approach introduced in #6129 reduced how
often these copies are made, but they're still frequent and very
expensive.
1. At no point does `SharedCache` have to re-serialize state to JSON
that it has already serialized. JSON serialization previously added
hundreds of milliseconds to any mutating operation on `SharedCache`.
As a result, this speeds up the initial load of the shared cache by
around 2x and loading of subsequent images improves by about the same.
One trade-off is that the serialization / deserialization logic is more
complicated. There are two reasons for this:
1. The state is now split across multiple metadata keys and needs to be
merged when it is loaded.
2. The in-memory representation uses pointers to identify memory regions.
These relationships have to be re-established after the JSON is
deserialized.
As a future direction it is worth considering whether the logic owned by
`SharedCache` could be split in a similar manner to the data. The
initial loading of the cache header, loading of images, and handling of
symbol information are all mostly independent and work on separate data.
If the logic were split into separate classes it would be easier to
reason about which data is valid when, and would easily permit
concurrent loading of multiple images from the shared library in a
thread-safe manner.
|
|
Previously, `MMappedFileAccessor::Open` attempted to impose a fixed limit
on the number of file accessors that were live at one time. This was
done because the default file descriptor limit on some platforms is
relatively low (256 on macOS). Given that recent iOS shared caches can
contain 60+ files it is easy to tie up a large percentage of this limit
by opening one or two shared caches.
This is problematic as if the limit imposed by `MMappedFileAccessor::Open`
is reached, the attempt to access the file will block waiting for
another file accessor to be closed. This can lead to a deadlock.
This commit makes three changes to this strategy:
1. It attempts to raise the file descriptor limit to 1024. Unix systems
support both soft and hard file descriptor limits. The soft file
descriptor limit is what is enforced, but a process can explicitly
raise the limit to any value below the hard limit if it wishes.
2. The fixed limit on open files accessors is changed to a soft limit.
`FileAccessorCache` is introduced to manage the caching of file
accessors. It provides a basic LRU cache. Whenever a new file is
opened, the cache of open accessors is pruned to stay below the
target limit (50% of the soft file descriptor limit). This limiting
is primarily done to allow files containing dirty pages to be
unmapped if they're no longer being used.
LRU isn't the optimal strategy for this, but it is simple to
implement and understand. Ideally there'd be some access time
component to the cache so files that haven't been accessed can be
released.
3. File accessors are cached even for sessions corresponding to views
that have been closed. The "Open Selection with Options..." context
menu hits this case as a view is created and destroyed as part of
populating the options dialog, and the real view is then created in
the same session.
The most significant benefit of this change is that the logic around
opening / closing file accessors is simpler and can no longer deadlock.
While working on this I noticed that `SharedCache` opened some files via
`MMappedFileAccessor::Open` without specifying a post-open operation to
apply slide information. Instead, it would explicitly apply the slide
information after opening the file. This works fine so long as the file
accessor limit is never hit. If it is hit and the accessor is closed,
the next time the file is opened it will not have any slide information
applied. This would lead to very confusing bugs.
|
|
This change is mostly motivated by simplifying the code, but it also
brings minor correctness and performance benefits.
1. The pointer returned by mmap is stored as a uint8_t* rather than
void* as that is how it is used. This reduces how often it needs to
be cast to a different type before it is used.
2. Read methods for primitives delegate to a new Read template function
that in turn delegates to the general-purpose `Read(void* dest, size_t
address, size_t length)`. This improves the consistency of bounds
checking and simplifies the code. The compiler is more than willing
to inline this so we get less repetition with no overhead.
3. ReadNullTermString now uses std::find to find the nul byte and
directly constructs the string from that range of bytes. This removes
an unnecessary allocation that was previously being forced by the use
reserve followed by shrink_to_fit. It also avoids repeated
reallocation for longer strings as they grew past the the reserved
size as they were being built up a character at a time.
|
|
`ReadExportNode` is called a lot during the initial load of the shared
cache and thus impacts how long it takes for the UI to become
responsive. This is a collection of optimizations that cut the time
spent within `ReadExportNode` by 50%:
1. Pass iterators to `ReadExportNode` rather than a `DataBuffer` + offset.
The lack of inlining in `DataBuffer`'s `operator[]` kills performance.
Ideally this would have used `std::span`, but that would require
bumping the minimum C++ version to C++20.
2. Add `MMappedFileAccessor::ReadSpan` so that `ReadExportNode` can
operate directly on the mapped data without first copying it.
3. Removes a call to `GetAnalysisFunctionsForAddress` whose result was
unused.
4. Use `std::find` to find the nul at the end of strings rather than
assembling the string a character at a time. This avoids repeatedly
growing the string.
5. Avoid the usual `Symbol` constructor in favor of `BNCreateSymbol`.
The `Symbol` constructor has over head in two forms:
1. It has a `NameSpace` as an argument. Creating / destroying this
allocates and deallocates memory We could create a single
instance and reuse it for all calls, but...
2. The constructor copies all fields of the `NameSpace` to the heap
before calling `BNCreateSymbol` and then deallocates them
afterwards. This seems unnecessary, and adds a non-trivial amount
of overhead.
This can go back to directly constructing the `Symbol` once the
constructors are improved.
|
|
`PageMapping` was storing the path to the file it points within. This
was causing unnecessary work within `VM::MappingAtAddress` as the
`PageMapping`, and thus the path, is copied into the return value. This
copying was more expensive than the map lookup.
The file path is now stored within a `LazyMappedFileAccessor` class that
wraps the `SelfAllocatingWeakPtr`. This means the path is still
available via the `PageMapping`, but it does not need to be copied as
often.
This includes two additional improvements / optimizations while I was
touching the code in question:
1. `MMappedFileAccessor::Open` no longer performs two hash lookups when
a file accessor already exists.
2. `VM::MapPages` takes the path by const reference to avoid an
unnecessary copy.
|
|
There are typically only a few dozen mappings, while there can be
millions of pages. This reduces the amount of time spent populating the
mapping from region to file accessor along with the memory usage of the
same.
|
|
`MMappedFileAccessor::ReadBuffer` was returning a heap-allocated
`DataBuffer`, but no callers were ever deleting it. There does not
appear to be any reason to heap allocate the `DataBuffer` as the type is
effectively a smart pointer wrapper around `BNDataBuffer`. Switch to
returning it by value instead.
Additionally, `MMappedFileAccessor::ReadBuffer` was allocating a buffer,
copying data into it, and then handing that allocation to the `DataBuffer`
constructor. The constructor copies data into a new allocation it
owns so this allocation is unnecessary and was being leaked.
|
|
|
|
This is an early release of our DSC processing plugin. We're still hard at work improving this feature. You should be able to just drop in a dyld_shared_cache and use the 'Shared Cache Triage' view to load and analyze images.
|
