[PATCH] LLD: Implement our own future and use that for FileArchive::preload().

[Rui Ueyama]

On Tue, Mar 3, 2015 at 11:24 AM, Shankar Easwaran <shankare at codeaurora.org>
wrote:

> On 3/3/2015 12:51 PM, Rui Ueyama wrote:
>
>> On Tue, Mar 3, 2015 at 8:53 AM, Shankar Kalpathi Easwaran <
>> shankarke at gmail.com> wrote:
>>
>>  ================
>>> Comment at: include/lld/Core/Parallel.h:77
>>> @@ +76,3 @@
>>> +/// https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01389.html
>>> +template<typename T> class Future {
>>> +public:
>>> ----------------
>>> there is only one usage of Future with lld::File, Why do we want to make
>>> this a template ?
>>>
>>
>> I didn't plan to implement a future. I first tried to do that all within
>> FileArchive using mutex and condition variables, and then realized that
>> that's too low-level multi-thread programming which is pretty error-prone.
>>
>> If you are familiar with multi-thread programming, futures are basic
>> building block of high-level multi-thread programming. It's a rendezvous
>> point of a producer thread and a consumer thread. I built a basic block
>> here. Replacing T of this Future class with File * is weird.
>>
>>
>>  ================
>>> Comment at: lib/ReaderWriter/FileArchive.cpp:63-69
>>> @@ -62,8 +62,9 @@
>>>       {
>>>         std::lock_guard<std::mutex> lock(_mutex);
>>>         auto it = _preloaded.find(memberStart);
>>>         if (it != _preloaded.end()) {
>>> -        std::future<const File *> &future = it->second;
>>> -        return future.get();
>>> +        std::unique_ptr<Future<const File *>> &p = it->second;
>>> +        Future<const File *> *future = p.get();
>>> +        return future->get();
>>>         }
>>>       }
>>> ----------------
>>> The _preloaded map could get resized here ? Can we resize the map in
>>> advance since we know how many members are in the archive ?
>>>
>>>  There's no such function like resize defined for std::map because of the
>> nature of the map. It's a balanced tree, there's no such notion like
>> making
>> room for future use.
>>
> I was hinting to make this a unordered_map which has a reserve function in
> it.


So, as I wrote in the previous mail, the number of items that the map will
contain is limited. I don't see a need to add more code to reserve room in
the map. I'm planning to do performance profiling and make things faster.
Let's not do micro-optimization at this moment.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150303/786ecc04/attachment.html>