Stream scripts to V8 as they load - Blink side.

Source/bindings/core/v8/V8ScriptStreamer.cpp

Index: Source/bindings/core/v8/V8ScriptStreamer.cpp
diff --git a/Source/bindings/core/v8/V8ScriptStreamer.cpp b/Source/bindings/core/v8/V8ScriptStreamer.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..23a8acd665a198d483fc72cd4e865e9130a55803
--- /dev/null
+++ b/Source/bindings/core/v8/V8ScriptStreamer.cpp
@@ -0,0 +1,340 @@
+/*
+ * Copyright (C) 2014 Google Inc. All rights reserved.

[haraken, 2014/08/17 16:05:27]

Nit: Use the 3-line copyright.

[marja, 2014/08/20 11:45:55]

Done.

+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "bindings/core/v8/V8ScriptStreamer.h"
+
+#include "bindings/core/v8/V8ScriptRunner.h"
+#include "bindings/core/v8/V8ScriptStreamerThread.h"
+#include "core/dom/Document.h"
+#include "core/dom/Element.h"
+#include "core/dom/PendingScript.h"
+#include "core/fetch/ScriptResource.h"
+#include "core/frame/LocalFrame.h"
+#include "core/frame/Settings.h"
+#include "core/html/parser/HTMLScriptRunner.h"
+#include "platform/SharedBuffer.h"
+#include "wtf/MainThread.h"
+#include "wtf/text/TextEncodingRegistry.h"
+
+#include <string.h>
+
+namespace blink {
+
+class LocalFrame;
+
+class SourceStream : public v8::ExternalSourceStream {

[haraken, 2014/08/17 16:05:26]

(I haven't reviewed this class yet. I'll take a closer look after other
high-level issues are resolved.)

[marja, 2014/08/20 11:45:55]

Acknowledged.

+public:
+    SourceStream(ScriptResource* resource)
+        : m_resource(resource)
+        , m_backgroundThreadWaitingForData(false)
+        , m_dataPointer(0)
+        , m_dataLength(0)
+        , m_dataPosition(0)
+        , m_finished(false)
+        , m_cancelled(false) { }
+
+    virtual ~SourceStream() { }
+
+    // Called by V8 on a background thread. Should block until we can return
+    // some data.
+    virtual unsigned GetMoreData(const char** src) OVERRIDE
+    {
+        MutexLocker locker(m_mutex);
+        if (m_cancelled) {
+            return 0;
+        }
+        // The main thread might be modifying the data buffer of the Resource,
+        // so we cannot read it here. Post a task to the main thread and wait
+        // for it to complete.
+        m_backgroundThreadWaitingForData = true;
+        m_dataPointer = src;
+        m_dataLength = 0;
+        callOnMainThread(WTF::bind(&SourceStream::getMoreDataOnMainThread, this));
+        m_condition.wait(m_mutex);
+        if (m_cancelled) {
+            return 0;
+        }
+        return m_dataLength;
+    }
+
+    // Called on the main thread.
+    void didFinishLoading()
+    {
+        if (m_resource->errorOccurred()) {
+            // The background thread might be waiting for data. Wake it up. V8
+            // will perceive this as EOS and call the completion callback. At
+            // that point we will resume the normal "script loaded" code path
+            // (see HTMLScriptRunner::notifyFinished which doesn't differentiate
+            // between scripts which loaded successfully and scripts which
+            // failed to load).
+            cancel();
+            return;
+        }
+        {
+            MutexLocker locker(m_mutex);
+            m_finished = true;
+            if (!m_backgroundThreadWaitingForData)
+                return;
+        }
+        getMoreDataOnMainThread();
+    }
+
+    // Called on the main thread.
+    void didReceiveData()
+    {
+        getMoreDataOnMainThread();
+    }
+
+    // Called on the main thread.
+    void cancel()
+    {
+        // The script is no longer needed. Stop streaming it. The next time
+        // GetMoreData is called (or woken up), it will return 0, which will be
+        // interpreted as EOS by V8 and the parsing will fail. Our "streaming
+        // complete" callback will be called, and at that point we will release
+        // the references to SourceStream.
+        MutexLocker locker(m_mutex);
+        m_cancelled = true;
+        m_backgroundThreadWaitingForData = false;
+        m_condition.signal();
+    }
+
+private:
+    // Called on the main thread.
+    void getMoreDataOnMainThread()
+    {
+        MutexLocker locker(m_mutex);
+        if (!m_backgroundThreadWaitingForData) {
+            // It's possible that the background thread is not waiting. This
+            // happens when 1) data arrives but V8 hasn't requested data yet, 2)
+            // we have previously scheduled getMoreDataOnMainThread, but some
+            // other event (load cancelled or finished) already retrieved the
+            // data and set m_backgroundThreadWaitingForData to false.
+            return;
+        }
+        if (!m_resourceBuffer) {
+            // We don't have a buffer yet. Try to get it from the resource.
+            SharedBuffer* buffer = m_resource->resourceBuffer();
+            if (!buffer) {
+                // The Resource hasn't created its SharedBuffer yet. At some
+                // point (at latest when the script loading finishes) this
+                // function will be called again and we will have data.
+                if (m_finished) {
+                    // The load finished and we never had a buffer. In normal
+                    // operations, this shouldn't happen, but it can probably
+                    // happen for empty scripts.
+                    m_backgroundThreadWaitingForData = false;
+                    m_condition.signal();
+                }
+                return;
+            }
+            m_resourceBuffer = RefPtr<SharedBuffer>(buffer);
+        }
+
+        if (m_dataPosition == 0) {
+            // This is the first time data is requested. Check the encoding.
+            const char* encoding = WTF::atomicCanonicalTextEncodingName(m_resource->encoding());
+
+            // Here's a list of encodings we can use for streaming. These are
+            // the canonical names.
+            if (strcmp(encoding, "windows-1252") == 0
+                || strcmp(encoding, "ISO-8859-1") == 0
+                || strcmp(encoding, "US-ASCII") == 0) {
+                SetEncoding(v8::ExternalSourceStream::ONE_BYTE);
+            } else if (strcmp(encoding, "UTF-8") == 0) {
+                SetEncoding(v8::ExternalSourceStream::UTF8);
+            } else {
+                // Otherwise, cancel the streaming. We don't stream two byte
+                // scripts to avoid the handling of byte order marks. Most
+                // scripts are Latin1 or UTF-8 anyway, so this should be enough
+                // for most real world purposes.
+                m_cancelled = true;
+                m_backgroundThreadWaitingForData = false;
+                m_condition.signal();
+            }
+        }
+
+        // Get as much data from the ResourceBuffer as we can.
+        const char* data = 0;
+        Vector<const char*> chunks;
+        Vector<unsigned> chunkLengths;
+        while (unsigned length = m_resourceBuffer->getSomeData(data, m_dataPosition)) {
+            // TODO(marja): Here we can limit based on the total length, if it

[haraken, 2014/08/17 16:05:27]

Nit: TODO(marja) => FIXME

[marja, 2014/08/20 11:45:55]

Done.

+            // turns out that we don't want to give all the data we have (memory
+            // vs. speed).
+            chunks.append(data);
+            chunkLengths.append(length);
+            m_dataLength += length;
+            m_dataPosition += length;
+        }
+        // Copy the data chunks into a new buffer, since we're going to give the
+        // data to a background thread.
+        if (m_dataLength > 0) {
+            char* copiedData = new char[m_dataLength];
+            unsigned offset = 0;
+            for (size_t i = 0; i < chunks.size(); ++i) {
+                memcpy(copiedData + offset, chunks[i], chunkLengths[i]);
+                offset += chunkLengths[i];
+            }
+            *m_dataPointer = copiedData;
+            m_backgroundThreadWaitingForData = false;
+            m_condition.signal();
+        } else if (m_finished) {
+            // We need to return data length 0 to V8 to signal that the data
+            // ends.
+            m_condition.signal();
+        }
+        // Otherwise, we had scheduled getMoreDataOnMainThread but we didn't
+        // have any data to return. It will be scheduled again when the data
+        // arrives.
+    }
+
+    ScriptResource* m_resource;
+
+    // For coordinating between the main thread and background thread tasks.
+    // Guarded by m_mutex.
+    bool m_backgroundThreadWaitingForData;
+    const char** m_dataPointer;
+    unsigned m_dataLength;
+    unsigned m_dataPosition;
+    bool m_finished;
+    bool m_cancelled;
+    RefPtr<SharedBuffer> m_resourceBuffer; // Only used by the main thread.
+
+    ThreadCondition m_condition;
+    Mutex m_mutex;
+};
+
+
+bool V8ScriptStreamer::startStreaming(PendingScript* script, HTMLScriptRunner* runner)
+{
+    if (V8ScriptStreamerThread::shared()->isRunningTask()) {

[haraken, 2014/08/17 16:05:26]

Given that we currently support only parser-blocking scripts, shouldn't this be
an ASSERT at the moment like this?

  ASSERT(!V8ScriptStreamerThread::shared()->isRunningTask());

[marja, 2014/08/20 11:45:55]

It can still be that we're already running a task. Experimentally, I got this
code path hit and afaict, it happens when multiple frames are loaded at the same
time (the script that hit this code was from an iframe). (Added a comment to
clarify it.)

+        // At the moment we only have one thread for running the tasks. A new
+        // task shouldn't be queued before the running task completes, because
+        // the running task can block and wait for data from the network.

[haraken, 2014/08/17 16:05:26]

Just to confirm: Is this the reason it's not easy to support not-parser-blocking
scripts?

[marja, 2014/08/20 11:45:55]

There are several modifications needed when we want to parse non-parser-blocking
scripts:
- The biggest, as you said, is here; we need to implement some kind of a thread
pool and find the optimal number of threads.
- The script waiting / async execution code path in HTMLScriptRunner needs to be
modified too.

All in all, I think streaming parsing blocking scripts is a reasonable start.
- This CL is big as is, I wouldn't want to add more features to the first
version.
- Parsing blocking scripts are the most critical scripts to get parsed fast; for
other scripts the trade-off will be less (as there might be some reasonable work
to be done while waiting for the script).

[haraken, 2014/08/20 15:00:18]

Understood.

+        return false;
+    }
+    ScriptResource* resource = script->resource();
+    if (!resource->url().protocolIsInHTTPFamily()) {
+        return false;
+    }
+    // We cannot filter out short scripts, even if we wait for the HTTP headers
+    // to arrive. In general, the web servers don't seem to send the
+    // Content-Length HTTP header for scripts.
+
+    LocalFrame* frame = script->element()->document().frame();
+    ScriptState* scriptState = ScriptState::forMainWorld(frame);

[haraken, 2014/08/17 16:05:26]

Just to confirm: Script streaming is used only in the main world, right?

[marja, 2014/08/20 11:45:55]

Only for parser blocking scripts of HTMLScriptRunner; that's the main world,
right?

[haraken, 2014/08/20 15:00:18]

Yes, I believe so. I also believe that non-parser-blocking scripts are
guaranteed to be executed in the main world as well.

+    if (scriptState->contextIsEmpty())
+        return false;
+    ScriptState::Scope scope(scriptState);
+
+    // script is a pointer to m_parserBlockingScript of HTMLScriptRunner, which
+    // might go out of scope if HTMLScriptRunner is destroyed. We will soon call
+    // PendingScript::setStreamer, which makes the PendingScript notify the
+    // V8ScriptStreamer when it is destroyed.
+    RefPtr<V8ScriptStreamer> streamer = adoptRef(new V8ScriptStreamer(script, runner));
+
+    v8::ScriptCompiler::CompileOptions compileOption = v8::ScriptCompiler::kNoCompileOptions;
+    if (frame->settings()) {
+        V8CacheOptions v8CacheOptions = frame->settings()->v8CacheOptions();
+        bool ignore;
+        V8ScriptRunner::CacheDecider(0, resource, v8CacheOptions, &streamer->m_cachedDataDataType, &compileOption, &ignore);

[haraken, 2014/08/17 16:05:25]

I don't fully understand why you can decide whether you should use V8
compilation cache or not at this point. Without loading the full source code, it
wouldn't be possible to make the decision.

This is one of my main concerns about this CL.

[marja, 2014/08/20 11:45:55]

(As discussed offline), this code path only decides what kind of cached data to
*produce*. Added a comment to clarify that.

+    }
+    v8::ScriptCompiler::ScriptStreamingTask* scriptStreamingTask = v8::ScriptCompiler::StartStreamingScript(scriptState->isolate(), &(streamer->m_source), compileOption);
+    if (scriptStreamingTask) {
+        // The background thread holds an implicit reference to streamer,
+        // because it's passed as a callback parameter. The corresponding deref
+        // is in V8ScriptStreamer::streamingComplete().
+        ScriptStreamingTask* task = new ScriptStreamingTask(scriptStreamingTask, streamer.get());
+        streamer->ref();
+        script->setStreamer(streamer.release());
+        V8ScriptStreamerThread::shared()->postTask(task);
+        return true;
+    }
+    return false;
+}
+
+void V8ScriptStreamer::notifyAppendData()
+{
+    m_stream->didReceiveData();
+}
+
+void V8ScriptStreamer::notifyFinished()
+{
+    m_stream->didFinishLoading();
+    m_loadingDone = true;
+    // The load complete notification can come before or after V8 has processed
+    // all the data. Send the notification after both loading and parsing have
+    // completed.

[haraken, 2014/08/17 16:05:26]

Can we create a helper method like notifyFinishedToHTMLScriptRunner()?

void notifyFinishedToHTMLScriptRunner() {
  if (m_v8Done && m_loadingDone)
    m_runner->notifyFinished(m_script->resource());
}

[haraken, 2014/08/17 16:05:26]

Help me understand: In what scenario is it possible that m_v8Done becomes true
before m_loadingDone becomes true? Does it happen only when the streaming
processing is cancelled explicitly?

[marja, 2014/08/20 11:45:55]

Done.

[marja, 2014/08/20 11:45:55]

The V8 side can finish before loading e.g., when V8 detects a parse error.
(Added a comment to clarify that.)

+    if (m_v8Done) {
+        m_runner->notifyFinished(m_script->resource());
+    }
+}
+
+void V8ScriptStreamer::streamingComplete()
+{
+    // It's possible that the corresponding PendingScript was deleted before V8
+    // finished streaming.
+    if (refCount() == 1) {

[haraken, 2014/08/17 16:05:25]

It's fragile to rely on the reference counter to judge if the PendingScript is
gone. Can you avoid using refCount() and instead use a more explicit way to
judge the fact?

[marja, 2014/08/20 11:45:55]

Done (I ended up needing a more explicit way anyway, when I fixed reloading
scripts).

+        deref();
+        return;
+    }
+
+    // We have now streamed the whole script to V8 and it has parsed the
+    // script. We're ready for the next step: compiling and executing the
+    // script.
+    m_v8Done = true;
+    // V8 no longer holds an implicit reference.
+    deref();
+
+    // The load complete notification can come before or after V8 has processed
+    // all the data. Send the notification after both loading and parsing have
+    // completed.

[haraken, 2014/08/17 16:05:26]

Ditto. Use notifyFinishedToHTMLScriptRunner().

[marja, 2014/08/20 11:45:55]

Done.

+    if (m_loadingDone) {
+        m_runner->notifyFinished(m_script->resource());
+    }
+}
+
+void V8ScriptStreamer::cancel()
+{
+    m_script = 0;
+    m_runner = 0;
+    // The streaming might still be ongoing. Tell SourceStream to try to cancel
+    // it whenever it gets the control the next time. It can also be that V8 has
+    // already completed its operations and our callback will be called soon.
+    m_stream->cancel();
+}
+
+V8ScriptStreamer::V8ScriptStreamer(PendingScript* script, HTMLScriptRunner* runner)
+    : m_script(script)
+    , m_runner(runner)
+    , m_stream(new SourceStream(script->resource()))
+    , m_source(m_stream) // Source takes ownership of m_stream.

[haraken, 2014/08/17 16:05:26]

BTW, "StreamedSource" and "SourceStream" are confusing. Can we rename to
something better?

StreamedSource => SourceStreamController ?

[haraken, 2014/08/17 16:05:26]

Given that StreamedSource has an ownership of the SourceStream, isn't there any
way to get the SourceStream from the StreamedSource?

Then we can remove the m_stream member from V8ScriptStreamer and write the above
code like this.

  m_source(adoptPtr(new SourceStream(script->resource())))

[marja, 2014/08/20 11:45:55]

I could add an accessor for getting the ExternalSourceStream* (abstract class)
from StreamedSource, but the caller would need to reinterpret_cast that to the
concrete SourceStream. Would you prefer that?

[marja, 2014/08/20 11:45:55]

By StreamedSource you mean v8::ScriptCompiler::StreamedSource, right?

StreamedSource is not a SourceStreamController... it doesn't control anything.
It's just a "script source code" object which happens to be streamed. Normally,
the thing given to v8::ScriptCompiler is v8::ScriptCompiler::Source (the source
code of a script) and then we compile that and get a v8::Script back.

So that's why I called v8::ScriptCompiler::StreamedSource like that, since
that's also a Source, it's just streamed. It contains other information
typically associated with script source code too, such as the cached data.

IOW, I'm willing to rename things in general, just that I don't think
SourceStreamController is the right name. :)

[haraken, 2014/08/20 15:00:18]

OK, then let's keep your current code as is.

[haraken, 2014/08/20 15:00:18]

Got it; StreamedSource makes sense.

However, it's still confusing that we also have "SourceStream" inherited from
v8::ExternalSourceStream. Can you come up with a better name than SourceStream?

[marja, 2014/09/09 17:00:26]

I tried to think of the naming a bit, but didn't figure out a better name yet. I
guess the V8 side should change too, so, instead of ExternalSourceStream it
would be something else..

+    , m_loadingDone(false)
+    , m_v8Done(false)
+{ }

[haraken, 2014/08/17 16:05:26]

Nit: Write { and } in separate lines.

[marja, 2014/08/20 11:45:55]

Done.

+
+} // namespace blink