Investigate JNA thread usage with Java callbacks about vlcj

[caprica]

Completed pending some further testing.

[caprica]

No ill effects observed with these changes.

[caprica]

See #100. This has been undone for now.

[caprica]

New version 3.5.0 of JNA supposedly has some changes for this. Reopening for investigation...

[caprica]

Maybe provide a system property to enable the different behaviour at the client application's request.

[hjohn]

Just noticed this as well. With JavaFX 8, the copying of the buffer to a JavaFX Canvas is working very well, even for full screen playback (1920x1080), but I did notice that 3 threads are being created for each frame (lock, display, unlock).

It doesn't seem to impact performance noticably on my setup, but it is a shame as it means threads are constantly created / destroyed making it hard to debug or keep track of important threads. I don't quite understand why JNA can't just pool these threads or threadwrappers.

[caprica]

Please see also the referenced issue #100 for some more information - I already tried to address this but I ended up with thousands of Thread instances that never got garbage collected - I think that is a worse situation.

[hjohn]

I read it, I was looking into it as well. I even asked Timothy Wall
about it, but he seems to think that atleast with JNA 3.5.0 this
shouldn't happen, see
https://groups.google.com/forum/#!topic/jna-users/95p-KeaoLxQ

On 31/07/2013 17:06, Mark Lee wrote:

Please see also the referenced issue #100
#100 for some more information

• I already tried to address this but I ended up with thousands of
  Thread instances that never got garbage collected - I think that is a
  worse situation.

—
Reply to this email directly or view it on GitHub
#52 (comment).

[caprica]

I just re-tested this.

You can either have the situation now with lots of short-lived Thread instances created which will all be properly garbage collected; or, if you enable the CTI, you get a guaranteed memory leak of two thread instances each time you play media.

Note also that my understanding is that the Thread instances are just proxies that get associated with a native thread as needed - consequently it has been suggested that there is no heavyweight overhead with this approach. In this sense, creating the Thread instance is no different from creating e.g. a temporary String or any other short-lived object.

I think a guaranteed memory leak is worse than the noisy Thread instance creation.

I would be very happy if this could be made to work with a CTI, but I don't really know what to do about it because of the leak that would be incurred.

[hjohn]

Yes, this is exactly what Timothy claims shouldn't happen. It might be a problem with how VLC handles their native threads. They could perhaps be using a pool and a new thread is used for each playback and the old one doesn't die but is re-added to the pool -- in that case the number of threads should be limited. Or perhaps VLC itself is leaking the threads.

I agree with you that a guaranteed memory leak is worse. I'll have to check what kind of overhead this 'lightweight' thread causes in the case of the lock/display/unlock for each frame, but I suppose it will be overshadowed by the frame buffer created at the same time.

[caprica]

The leak could be a problem with the way I've implemented it in vlcj. I have to admit I've not really looked into this in great detail.

[caprica]

As part of the major refactoring for vlcj-4 under #690, I have used CallbackThreadInitializers. The thread model as a whole has changed, so maybe it's OK this time.

This issue will remain open for now as a reminder to test what happens with vlcj-4.

[caprica]

I have a nagging doubt that right now even with the new implementation this is still going to be a problem - e.g. each time you play new media and add a listener, there may be an undetached unused thread left over.

Needs some careful testing.

At least now with the media and media player handling their own events separately, this issue might only affect changing media - and the CTI could be removed in that case but remain in the media player case.

[caprica]

Some initial profiling results... bearing in mind that no listeners are attached to the media event manager.

It is encouraging but not 100% clear.

Even after forcing garbage collections, it first appeared that each time media was started, a new CTI was created and not cleared - however, after waiting a little longer those CTI did indeed appear to be garbage collected.

Further testing is required, and especially for the case when adding listeners to the media (as this involves another event manager).

[caprica]

Even though no listeners are being attached to the media event manager in the current implementation, the callback thread is still being created.

I am seeing inconsistent results, maybe attributable to caching, as what appears to happen now is when repeatedly playing new media you will get a new CTI created until you get, say, 10 of them, at which point some of them (not all of them) will disappear.

So presently it appears to be the case that the CTI instances will grow for a time before eventually being cleared away leaving 2 CTI instances related to the media event manager. It will then grow again, and eventually come back to leaving 2 CTI instances.

I can change the implementation so that CTI is not needed until the listener is added, which would be a Good Thing to do.

Further testing is still needed when a media listener is added.

[caprica]

I changed the implementation at 4be8e3c to only create the callback and the CTI if there was at least one listener, and to explicitly invoke detach on the CTI when all the listeners were removed.

[caprica]

Latest test adds a listener to each media as it is played (and makes sure to remove it at the appropriate time).

Inspecting the heap dump shows no ongoing increase in CTI instances.

It bizarrely seems to keep one extra, which I don't quite understand, but the good news is there is no apparent long term leak.

So finally I am happy to close this issue as the new implementation for vlcj-4 seems sound.

The outstanding issue of the seemingly 1 "extra" instance hanging around can be looked at in a more general profiling task later, it could be attributed to pooling, I'm not sure.