Closed Bug 1631270 Opened 6 years ago Closed 1 year ago

IE test Drive: Web Workers Fountain demo is slow

Categories

(Core :: JavaScript Engine: JIT, defect, P3)

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: mayankleoboy1, Unassigned)

References

(Blocks 1 open bug)

Details

  1. Go to https://testdrive-archive.azurewebsites.net/Graphics/WorkerFountains/Default.html
  2. Select the following demo : "Battle on the water (5 fountains)"
  3. Unselect "Use web workers"

ER: fast
AR: slow

Profile: https://perfht.ml/2VlAfhG

Component: JavaScript Engine: JIT → JavaScript: GC

(In reply to Mayank Bansal from comment #0)
The top line of the memory profile that shows 700-900ms GCMajor slices is indicating the total duration of incremental GCs, not the time spent doing GC (which is the line just below it). The profile shows 0.5% of the time spent in GCRuntime::collect, which is not that much.

The memory use seems to be generally increasing despite the fact that we're doing GC, so perpaps the page is leaking.

The profile does spend 17% of its time in js::NativeSetProperty. I don't know whether that is worth looking into.

Component: JavaScript: GC → JavaScript Engine
Summary: IE test Drive: Web Workers Fountain demo is slow (700-900ms in GC) → IE test Drive: Web Workers Fountain demo is slow

The FlameGraph view of the profile is quite talkative.
Most of the time seems to be spent under Baseline + CacheIR. Maybe we could have Ion-compiled this function sooner.

Jan, Matthew, any idea if there is a low hanging fruit here?

Type: task → defect
Component: JavaScript Engine → JavaScript Engine: JIT
Flags: needinfo?(mgaudet)
Flags: needinfo?(jdemooij)
Priority: -- → P3

The profile has us spending time doing slow SetElems under the updatePoints JS function. That JS function calls writeCoord where a comment mentions sparse arrays...

    // Draws the point to the frameArray, a sparce array of RGBA values and the _key_ location for the canvas
    Point.prototype.writeCoord = function (frameArray) {

I'm not sure if this is the problem though...

Flags: needinfo?(jdemooij)
Flags: needinfo?(mgaudet)
Severity: -- → S3

FWIW, profile with latest Nightly : https://share.firefox.dev/3uCgyDf

In this profile, we're spending 11% of our time atomizing property names for GetElem / SetElem.

The hot GetProp is in this function:

    function messageEvtEmulator(rawMessage) {
        // SCA simulation
        var frames = rawMessage.pckage.frames;
        var copy = [];
        for (var i = 0, len = frames.length; i < len; i++) {
            var frame = frames[i].rgbaCoords;
            for (x in frame)
                copy[x] = frame[x];
            frames[i].rgbaCoords = copy;
        }
        
        var dataEmulator = { data: rawMessage };
        callback(dataEmulator);
    }

I assume that it must be copy[x] = frame[x].

The hot SetProp is in this function

// This is my drawing loop (paints the canvas at [optimally] 60 fps)
function renderLoop() {
    if (!useWorkers) // Make a demand for the next [complete] frame (it will also be marked as ready)
        worker.postMessage(new Msg(CODE_REQUESTFRAME)); // synchronous in UI-thread model (generates the frame data and adds it to the frameQueue)
    
    // Check if I:
    // 1. have a frame in the frameQueue
    if (frameQueue.length > 0) { // There's a [ready] frame in the queue (at least one)
        var arrayMap = frameQueue.shift();
        for (var arrayMapIndex in arrayMap)
            pixelArray[arrayMapIndex] = arrayMap[arrayMapIndex];
        
        // Now the pixelArray is updated; put it's imageData back onto the canvas
        targetCanvasContext.putImageData(imageData, 0, 0);
        // Clear the prior image from the main canvas, and draw the target to the main canvas.
        canvasContext.clearRect(0, 0, canvas.width, canvas.height);
        canvasContext.drawImage(targetCanvas, 0, 0, canvas.width, canvas.height);

        // Only count a "frame" if there's a frame to render. Otherwise, an entire frame interval is lost (rather than slightly delayed if it came late)
        fpsUpdate();
    }

    // Manage the generator (for the web-worker case)
    if (useWorkers && (frameQueue.length < FRAME_QUEUE_MINIMUM_SIZE) && !generatorRunning) {
        worker.postMessage(new Msg(CODE_CONTINUE));
        generatorRunning = true;
    }
    else if (useWorkers && (frameQueue.length > FRAME_QUEUE_MAXIMUM_SIZE) && generatorRunning) {
        worker.postMessage(new Msg(CODE_PAUSE));
        generatorRunning = false;
    }

    bufferUpdate(); // DEBUG

    stepAnimationLoop();
}

Note that it also contains a loop of the form for (var x in old) new[x] = old[x].

It's weird that these values aren't already atoms; I was under the impression that the property iterator we create for the for-in should only be returning pre-atomized property key strings, but maybe that's not correct.

(In reply to Iain Ireland [:iain] from comment #5)

It's weird that these values aren't already atoms; I was under the impression that the property iterator we create for the for-in should only be returning pre-atomized property key strings, but maybe that's not correct.

Not for integer keys. The profile suggests it's using sparse elements...

Web workers + High resolution : https://share.firefox.dev/47xKXVs
No web workers + High resolution: https://share.firefox.dev/3qz2743

No web workers + No high resolution : https://share.firefox.dev/3OUFYqw
Web workers+ No high resolution: https://share.firefox.dev/3QCghvX

The demo doesnt seem to exist anymore.
Edit: Seems to be here: https://github.com/MicrosoftEdge/Demos-old

Demo does not exist anymore.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.