Khronos Public Bugzilla
Bug 817 - Add details about the release method usage
Add details about the release method usage
Status: RESOLVED FIXED
Product: WebCL
Classification: Unclassified
Component: Specification
1.0
PC Mac OS
: P2 major
: ---
Assigned To: WebCL Mailing List
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-28 12:16 PST by Antonio Gomes
Modified: 2013-09-18 07:54 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio Gomes 2013-02-28 12:16:29 PST
JS code:

var A = new WebCLXXX();
var B = A; (not a deep copy, so the same opencl object reference is held by both)
A.release();

Is A null?
Is A not null but holding a null'ed correcting opencl object?
If the opencl object held by A had its ref count manually increased at some point before 'release' is called, will 'release' just deref it and not actually release it? will the JS programmer know it did not actually released memory?
Is B still a valid object?
Is B valid, but holding a dangle reference to the corresponding opencl object?

We are change our WebKit implementation to deal with such cases, but the spec could be clearer about it as well(?)
Comment 1 Tomi Aarnio 2013-03-07 08:02:20 PST
(In reply to comment #0)

> var A = new WebCLXXX();
> var B = A;
> A.release();

My understanding is that both A and B will continue to be defined, non-null, and pointing to the same WebCL object. The WebCL object that they're referencing, however, will be "neutered", which means that calling any of its member functions should probably throw an exception.

Then what type of exception should we throw? I tried to look for precedents in the TypedArray spec, but it turns out they haven't yet specified what happens if you try to use an ArrayBuffer that's been neutered.

Let's leave this open until we've figured out the type of exception to use, and written that down in the spec.
Comment 2 steven eliuk 2013-03-13 15:54:01 PDT
There was a long discussion during the meetup concerning this bug on 03-13-13 and the WG agreed there should be a section in the WD that defines the release mechanism in more detail. 
Likewise, the 'native' statement in the spec would be revised.
Comment 3 steven eliuk 2013-04-03 14:04:21 PDT
Would any be opposed to the addition of the following statement to the specification?

"after release is called there is no async-callbacks. Best practices suggest if an application is reliant on the callbacks then one should wait for all call-backs before calling release."

Likewise, should we explicitly state that if an object was created based on a parents definition, then that object is then released if the parent object was released?

ie) if one releases the context, any commandqueue, image, program, etc is not longer valid.
Comment 4 steven eliuk 2013-04-04 11:04:27 PDT
This bug will be used to continue the conversation concerning release functionality,

There will also be two new bugs created, they are:
-What is the expected behavior of callbacks after release is called on a context, etc? Now Bug 847,
-What exception is thrown if the application tries to access a resource that has been released? Now Bug 848,
Comment 5 Antonio Gomes 2013-05-13 09:36:04 PDT
(In reply to comment #4)
> This bug will be used to continue the conversation concerning release
> functionality,
> 
> There will also be two new bugs created, they are:
> -What is the expected behavior of callbacks after release is called on a
> context, etc? Now Bug 847,
> -What exception is thrown if the application tries to access a resource that
> has been released? Now Bug 848,

1) What should happen to clgl_context if one does:

var clgl_context = device.getExtension("cl-gl").getContext({"device":device});
device.release();

I would think clgl_context should not be neutered but any attempt to use it should throw an invalid_device exception, since the device used to create it was neutrered. Does it make sense?

In fact, likely, OpenCL itself will take care of throwing an invalid_device error.

Should the spec mention anything about such cases, since context is not created by device, but instead referencing the device.

2) Same thing for WebCLContext::createCommandQueue(optional WebCLDevice?, ...);
Comment 6 Antonio Gomes 2013-05-13 10:32:43 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > This bug will be used to continue the conversation concerning release
> > functionality,
> > 
> > There will also be two new bugs created, they are:
> > -What is the expected behavior of callbacks after release is called on a
> > context, etc? Now Bug 847,
> > -What exception is thrown if the application tries to access a resource that
> > has been released? Now Bug 848,
> 
> 1) What should happen to clgl_context if one does:
> 
> var clgl_context =
> device.getExtension("cl-gl").getContext({"device":device});
> device.release();
> 
> I would think clgl_context should not be neutered but any attempt to use it
> should throw an invalid_device exception, since the device used to create it
> was neutrered. Does it make sense?
> 
> In fact, likely, OpenCL itself will take care of throwing an invalid_device
> error.
> 
> Should the spec mention anything about such cases, since context is not
> created by device, but instead referencing the device.
> 
> 2) Same thing for WebCLContext::createCommandQueue(optional WebCLDevice?,
> ...);


3) More general: is the creator object of an extension (say a 'device') gets garbage collected, what should it happen to the extension object and objects created off of it?

function bleh() {
   var platform = webcl.getPlatforms()[0];
   var device = platform.getDevices()[0];
   var context = device.getExtension("cl-gl").createContext();
   return context;
}
Comment 7 Tomi Aarnio 2013-05-14 03:49:52 PDT
(In reply to comment #5)

> device.release();

There's no release() method in WebCLDevice. In both WebCL and OpenCL, devices are kind of static properties that are not created or released dynamically. The same applies to WebCLPlatform.
Comment 8 Tomi Aarnio 2013-05-15 02:47:05 PDT
Looks like we've resolved all open issues in this ticket, so I'm closing it.
Comment 9 Antonio Gomes 2013-08-05 21:58:47 PDT
(In reply to comment #3)
> (...)
> Likewise, should we explicitly state that if an object was created based on
> a parents definition, then that object is then released if the parent object
> was released?
> 
> ie) if one releases the context, any commandqueue, image, program, etc is
> not longer valid.

OpenCL 1.2 spec says the opposite: "clReleaseContext decrements the context reference count (...). After the context reference count becomes zero and all the objects attached to context (such as memory objects, command - queues) are released, the context is deleted."

So releasing a parent object only actually takes effect when all children are manually released. 

On the other hand, the current WD states that releasing a parent object will implicitly release all its descendants, which is different from OpenCL's release:

"2.1.3 Resource management

All dynamically created WebCL objects contain a release method that releases the memory and other resources consumed by that object, as well as the resources of its descendant objects (if any). This is illustrated in the following example:

  var ctx1 = WebCL.createContext(...);
  var ctx2 = WebCL.createContext(...);
  var A = ctx1.createBuffer(...);
  var B = ctx2.createBuffer(...);
  var C = A;
  var D = B;
  A.release();     // releases A and C
  ctx2.release();  // releases ctx2, B, and D
"
Comment 10 Tomi Aarnio 2013-08-06 06:18:14 PDT
(In reply to comment #9)
> On the other hand, the current WD states that releasing a parent object will
> implicitly release all its descendants, which is different from OpenCL's
> release:

Do we agree that the browser must be able to release all CL objects when the page is destroyed, so as to avoid leaking memory? If yes, then it follows that the browser must keep track of CL object allocations.

Now, since the browser is already keeping track of CL objects, do we want to force the JavaScript developer to also keep track of them? Moreover, since the browser must be able to release CL objects automatically in one situation, why should it not do that in other situations that are substantially similar?

(The generic principle of "we should align with OpenCL" no longer applies if we require WebCL to clean up whatever mess the application leaves behind. OpenCL has no such requirement.)
Comment 11 Antonio Gomes 2013-08-06 06:47:00 PDT
@Tommi, valid points.

Below is what WebGL says:

"3 WebGL Resources

OpenGL manages several types of resources as part of its state. These are identified by integer object names and are obtained from OpenGL by various creation calls. In contrast WebGL represents these resources as DOM objects. Each object is derived from the WebGLObject interface. Currently supported resources are: textures, buffers (i.e., VBOs), framebuffers, renderbuffers, shaders and programs. The WebGLRenderingContext interface has a method to create a WebGLObject subclass for each type. Data from the underlying graphics library are stored in these objects and are fully managed by them. The resources represented by these objects are guaranteed to exist as long as the object exists. Furthermore, the DOM object is guaranteed to exist as long as the author has an explicit valid reference to it or as long as it is bound by the underlying graphics library. When none of these conditions exist the user agent can, at any point, delete the object using the equivalent of a delete call (e.g., deleteTexture). If authors wish to control when the underlying resource is released then the delete call can be made explicitly."

It does not mention clearly the parent x children object deallocation either, i.e. if a gl-context is explicitly deleted, do all objects created off of it get implicitly deleted?
Comment 12 Tomi Aarnio 2013-08-06 07:04:57 PDT
(In reply to comment #11)
> It does not mention clearly the parent x children object deallocation
> either, i.e. if a gl-context is explicitly deleted, do all objects created
> off of it get implicitly deleted?

I don't think there's any means to explicitly delete a WebGLRenderingContext, but when it's implicitly deleted (such as when the user hits 'reload'), the browser is supposed to delete all GL objects created from it. (See also Bug 921.)
Comment 13 Antonio Gomes 2013-08-06 08:03:20 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > It does not mention clearly the parent x children object deallocation
> > either, i.e. if a gl-context is explicitly deleted, do all objects created
> > off of it get implicitly deleted?
> 
> I don't think there's any means to explicitly delete a
> WebGLRenderingContext, but when it's implicitly deleted (such as when the
> user hits 'reload'), the browser is supposed to delete all GL objects
> created from it. (See also Bug 921.)

You are right. WebGL's context is not a WebGLObject, and its hierarchy is different than WebCL's.

Our deletion could go top-down, but as you said, it would require implementors to keep track of object creation instead of JS programmers.

WD could mention it differs from OpenCL in this case.
Comment 14 Mikael Bourges-Sevenier 2013-09-11 15:13:00 PDT
Resolution:
- keep the behavior of release() as it is in the spec.
- release() behaves differently from OpenCL releaseXXX() methods
1) OpenCL release() decrements a reference count on an object
2) WebCL release() destroys underlying resources and this includes children resources. In essence it releases the memory used by a WebCLObjects
Comment 15 Mikael Bourges-Sevenier 2013-09-11 15:32:55 PDT
Note: releasing resources in WebWorkers

Currently only WebCLMemoryObject are transferable. This means that a WebCLContext must be created in the WebWorker and it may create WebCLMemoryObject that could be returned to the main application. In that case, 
- ownership of the returned WebCLMemoryObjects will be transfered to the main application,
- WebWorker's WebCLContext will be release() hence destroying it and its children, except the transfered WebCLMemoryObjects
Comment 16 Jeff Gilbert 2013-09-11 16:09:19 PDT
`release()` is useful in the context of `retain`, such as from `clRetainContext`.

Assume app A is passing a WebCL context to library B, calling `retain` as it does. On completion of their work, both A and B can call `release`, and the final (here, second) one to do so (in any order) triggers attempting destruction of the underlying context.

Note that this is distinct from JS's notion of garbage collection of orphaned objects, and also separate from OpenCL's internal refcounts. This means that we do not need to worry about an app calling `retain` or `release` repeatedly, as these will neither cause an object to be held too long, nor be destroyed too early.

This functionality is not strictly necessary in a garbage-collected language like JS, but it could improve resource utilization if we allow for more eager freeing of resources.

Since this retain/release API is not necessary in JS, we should determine whether we want to retain (hah) it in WebCL. If we choose not to, an app (even a port) should get the same behavior by just relying on JS's internal garbage-collection.
Comment 17 Mikael Bourges-Sevenier 2013-09-11 16:12:38 PDT
Note: releasing resources in WebWorkers

Currently only WebCLMemoryObject are transferable. This means that a WebCLContext must be created in the WebWorker and it may create WebCLMemoryObject that could be returned to the main application. In that case, 
- ownership of the returned WebCLMemoryObjects will be transfered to the main application,
- WebWorker's WebCLContext will be release() hence destroying it and its children, except the transfered WebCLMemoryObjects
Comment 18 Tomi Aarnio 2013-09-12 05:37:58 PDT
(In reply to comment #16)
> `release()` is useful in the context of `retain`, such as from `clRetainContext`.

It looks to me that we're having this same conversation in Bug 942, which was just closed with a decision to not support clRetainContext. Was that the final decision, or should we mark Bug 942 as a duplicate of this bug and continue the discussion?
Comment 19 Antonio Gomes 2013-09-12 06:22:01 PDT
(In reply to comment #18)
> (In reply to comment #16)
> > `release()` is useful in the context of `retain`, such as from `clRetainContext`.
> 
> It looks to me that we're having this same conversation in Bug 942, which
> was just closed with a decision to not support clRetainContext. Was that the
> final decision, or should we mark Bug 942 as a duplicate of this bug and
> continue the discussion?

Right. This bug should be reclosed, I would say.
Comment 20 Antonio Gomes 2013-09-12 19:17:17 PDT
A some points that have been discussed during the F2F:

- It might not be interesting to have a "release" method that does not correspond to the OpenCL "release" method, in terms of behavior.

- It might not be interesting to have WebCL's "release" matching OpenCL's "release", without have a associated WebCL "Retain" API matching OpenCL's "retain".

- The existing recursive WebCL "release" method, could be an WebCL extension  called "delete", given its behavior.