[PATCH] Speculatively instantiate archive members

Rui Ueyama ruiu at google.com
Fri Jan 16 14:32:50 PST 2015


================
Comment at: lib/ReaderWriter/FileArchive.cpp:274
@@ +273,3 @@
+  mutable std::map<const char *, std::future<const File *>> _preloaded;
+  mutable std::vector<std::unique_ptr<std::promise<const File *>>> _promises;
+  mutable std::mutex _mutex;
----------------
atanasyan wrote:
> ruiu wrote:
> > atanasyan wrote:
> > > Can we store promises directly and do not use `unique_ptr` wrapper? Though I did not test the following code, it compiled successfully. Does it have a sense?
> > > 
> > > ```
> > > // Instantiate the member
> > > _promises.emplace_back();
> > > auto &promise = _promises.back();
> > > _preloaded[memberStart] = promise.get_future();
> > >                                                
> > > group.spawn([this, &promise, ci] {
> > >   std::unique_ptr<File> result;
> > >   if (instantiateMember(ci, result)) {
> > >     promise.set_value(nullptr);
> > >     return;
> > >   }
> > >   promise.set_value(result.release());
> > > });
> > > 
> > > [...]
> > > 
> > > mutable std::vector<std::promise<const File *>> _promises;
> > > ```
> > I think promises created here need to be deleted when it's no longer needed. Your code wouldn't delete promises, no?
> They will be destroyed at the same time with the `_promises` container as any other types own destructor and stored in the `std::vector`.
I made a change as you suggested but eventually decided to roll it back because it didn't work on Windows. I don't fully understand why it sometimes raised an exception -- there might be a bug in std::future's move constructor? Anyways, the cost of referencing futures through unique_ptr should be negligible here.

http://reviews.llvm.org/D7015

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list