[PATCH] getStreamedBitcodeModule: take StreamingMemoryObject instead of DataStreamer

[Jan Voung]

On Thu, Apr 24, 2014 at 6:02 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> On 18 April 2014 14:14, Jan Voung <jvoung at chromium.org> wrote:
> > jvoung added you to the CC list for the revision
> "getStreamedBitcodeModule: take StreamingMemoryObject instead of
> DataStreamer".
> >
> > Currently, getStreamedBitcodeModule takes a DataStreamer and then
> > creates a StreamingMemoryObject for the BitcodeReader. That allows
> > flexibility in the supplied DataStreamer implementation but does
> > not allow flexibility in the StreamingMemoryObject implementation.
> >
> > For example, the StreamingMemoryObject might want to cache bytes
> > in a thread-local way.
> >
>
> Do you have an example where that would not be the case? That is, why
> not just add the features you need to DatStreamer?
>

Hi Rafael,

DataStreamer's interface is pretty much just one method that "pulls N
bytes" from the current position and advances the current position for the
next pull. So it doesn't have random access.

On the other hand, the StreamableMemoryObject interface adds random access
and it is the interface that the bitcode reader interacts with. Given that,
it made more sense for us to have a thread cache at the random-access level
than at the DataStreamer stream level.

If it helps clarify things, the way we have an out-of-tree implementation
set up is to have:

(*) One instance of a DataStreamer to pull bytes from. It gets bytes from
an URL downloader and may block if bytes haven't been downloaded yet
(involves two threads).

(*) One plain StreamingMemoryObject(Impl) instance talking to the
DataStreamer, to buffer things from the DataStreamer and provide basic
random access.

(*) N instances of thread cached memory objects that also implement the
StreamingMemoryObject interface, wrapping the plain StreamingMemoryObject
instance to provide some thread local caching and thread safety, while also
providing random access.

(*) N threads w/ N contexts and N modules materialized by N bitcode readers
that each get one of the N thread cached StreamingMemoryObjects (the
bitcode reader is able to use the thread cached memory objects instead of
the plain non-thread-safe memory objects, because of this refactoring).



> In general having an interface with just one implementation is undesirable.


Yes, I can see that as not being too desirable, but does the above make
sense as to why the interface is separated out from the implementation and
why we don't add the features to the DataStreamer instead?


> Cheers,
> Rafael
>

Thanks!
- Jan


> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140425/d01ee800/attachment.html>