[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