gpujs / gpu.js

GPU Accelerated JavaScript
https://gpu.rocks
MIT License
15.03k stars 643 forks source link

Nigh unstoppable memory leaks from repeated application of immutable pipelined textures (Texture.delete() does not seem to work) #815

Closed Aloeminium108 closed 1 year ago

Aloeminium108 commented 1 year ago

What is wrong?

While other textures seem to be deleting properly, if I apply the same immutable pipelined kernel over and over again, it causes a memory leak.

Where does it happen?

I've been using GPU.js version 2.16.0 (though reverting to other versions has not helped) in an app that I've been testing in Chromium and Firefox with Browserify on Arch Linux. The app is written in TypeScript and uses Express/React.

How do we replicate the issue?

WARNING: this code causes a memory leak, execute at your own risk; reducing the size or numIterations variables will change the size of the leak. Also, in my use case, this code is being executed once a frame. In order to diagnose the issue without causing Chromium to crash, I've capped the framerate in my app to one frame a second. So, the loop that causes the issue is being executed once every second. (Creating the GPU and the kernels happens outside of the animation frames, so they are not the source of the leak)

const size = 256
const numIterations = 30

const gpu = new GPU()

const generateInput = gpu.createKernel(function() {
   return [0, 0]
})
   .setOutput([size, size])
   .setPipeline(true)

const kernel = gpu.createKernel(function(input: number[][][]) {
   const result = input[this.thread.y][this.thread.x]
   return [result[0] + 1, result[1] + 1]
})
   .setOutput([size, size])
   .setPipeline(true)
   .setImmutable(true)
   .setArgumentTypes({ input: 'Array2D(2)' })

const input = generateInput()
kernel(input)

let frameCount = 0
let prevTimeStamp = Date.now()

const animate = () => {

    frameCount++

    const timeStamp = Date.now()

    if (timeStamp - prevTimeStamp >= 1000) {
        prevTimeStamp = timeStamp
        frameCount = 0
    }

    if (frameCount === 1) {

        // Loop where memory leak happens
        for (let x = 0; x < numIterations; x++) {
            let prevPass = kernel.texture
            kernel(kernel.texture)
            prevPass.delete()
        }

    }

    requestAnimationFrame(animate)

}

I will note that changing the line from

const prevPass = kernel.texture

to

const prevPass = kernel.texture.clone()

makes no difference.

I have gone in circles attempting everything under the sun to fix this. I can believe that there's something I haven't tried yet, but I would be genuinely surprised. There are only two things that I've found that have seemed to help, but both of those solutions lead me to believe there maybe an issue with GPU.js itself.

How important is this (1-5)?

To me this is a 5. The algorithm that I'm trying to use in the actual app just can't work without repeated passes in some way. I haven't been able to find any workaround that will produce the same result while avoiding the memory leak. Also, since the loop needs to be called every frame, I need the kernel to be pipelined.

Expected behavior (i.e. solution)

The expected behavior is for there to be no memory leak from a loop like this, so long as the textures are deleted appropriately.

As mentioned before, there are two "solutions" I've found, but neither are very satisfactory.

The first "solution" is to go into into the module itself and modify it. In gpu.js/src/backend/gl/texture/index.js there is a block of code:

  delete() {
    if (this._deleted) return;
    this._deleted = true;
    if (this.texture._refs) {
      this.texture._refs--;
      if (this.texture._refs) return;
    }
    this.context.deleteTexture(this.texture);
    // TODO: Remove me
    // if (this.texture._refs === 0 && this._framebuffer) {
    //   this.context.deleteFramebuffer(this._framebuffer);
    //   this._framebuffer = null;
    // }
  }

After deleting (in practice I've only ever commented it out) the if (this.texture._refs)... block so it looks like this:

  delete() {
    if (this._deleted) return;
    this._deleted = true;
    this.context.deleteTexture(this.texture);
    // TODO: Remove me
    // if (this.texture._refs === 0 && this._framebuffer) {
    //   this.context.deleteFramebuffer(this._framebuffer);
    //   this._framebuffer = null;
    // }
  }

Suddenly the memory leak is gone.

The second "solution" I've found is to attempt to delete the textures from previous passes more forcefully by changing the loop to

for (let x = 0; x < numIterations; x++) {
   const prevPass = kernel.texture
   kernel(kernel.texture)
   kernel.context.deleteTexture(prevPass.texture)
}

Of course, that internal texture field is not public in Texture (At least in the version of GPU.js I'm using), so the TypeScript compiler doesn't like this. But it does seem to work. I saw that doing Texture.clear() also decrements _refs, so I've tried doing prevPass.clear() before prevPass.delete() but it doesn't seem to work.

Other Comments

If the real issue is that I'm using something wrong, then I would greatly appreciate any advice on how to get this to work without a memory leak.

svengcz commented 1 year ago

I have somehow the problem with mutable kernels. I need also to upload big data from time to time as textures to process them in several kernels. My problem is that it is not a real memleak, but the memory is released to the operating system really late.

Did you find a solution for your problem?

Aloeminium108 commented 1 year ago

I just figured out how to fix my issue a few hours ago, and it turns out the problem for me wasn't any bug causing a memory leak. Rather, the issue was that the way that I should've have been freeing memory wasn't working because of the way I was using mutable and immutable kernels together.

If you'll notice, the block of code in the for loop looks like this:

let prevPass = kernel.texture
kernel(kernel.texture)
prevPass.delete()

This actually deletes the wrong thing. What the loop should look like is:

let prevResult = generateInput() as Texture
let result: Texture
for (let x = 0; x < numIterations; x++) {
   result = kernel(prevResult) as Texture
   prevResult.delete()
   prevResult = result
}

The issue is that I'm trying to delete the input to the kernel as it is no longer useful, but instead I am deleting the kernel's previous output. Since it's an immutable kernel, I assumed these were the exact same thing, but it seems they are not. Immutable kernels write to a clone of the input texture, so what is essentially happening is that every loop I am cloning the previous result, deleting one of those textures, but still leaving the other floating around. That causes the leak. (I sincerely apologize if this explanation is at all inaccurate. This is the best guess that a friend and I have been able to come up with after many, many hours of debugging, tracing, profiling, and reading source code)

Now, as I've said, I tried everything before posting this. If you were to copy and paste what I put in the original post with the now amended for loop, it probably won't work. By the time I had submitted this issue, I had tried to delete the textures the correct way several times, but it would never run. It always gave me the following error:

WebGL: INVALID_OPERATION: bindTexture: attempt to use a deleted object

At this point I don't feel confident to even speculate what is going on exactly, but in this toy example the only way I could get it to run without that error was to pull

let prevResult = generateInput() as Texture

out of the animate() callback. Doing that, for some reason, fixed the issue. In my actual app, it was nowhere near as simple, and the solution ended up being to make most of the kernels immutable. Something about the transition from a mutable kernel to an immutable one was causing things to behave in ways that I didn't expect. Perhaps someone with more knowledge of the codebase could give a real explanation, but finding those transitions and spackling over them with .setImmutable(true) was what worked for me. Not sure if this is of any help to anybody but I still feel obligated to report to the best of my ability what I've learned in case it is.

My app now runs beautifully, by the way. I almost can't believe how fast it's running, considering attempts by other people to do similar things haven't even come close.

svengcz commented 1 year ago

Ok, I assume this is not my problem. My generateInput function uploads data to the GPU to have a Texture cache available for other kernels.

My current assumption is that the memory reallocations happen somewhere during the uploads.