-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Find a way to stop misbehaving plugins from slowing down driver #98
Comments
This is an interesting question. I don't think reads of the parameter library are atomic, at least not for string parameters? The lock is protecting all of the class data, including the parameter library, private member data, etc. In practice it is not too hard to write a plugin that unlocks during any time consuming operations. This sometimes requires reading the parameter library at the start and caching the values in local variables, and then not writing to the parameter library until the end after the lock is taken again. That being said, yesterday I discovered and fixed a serious performance problem with the base class NDPluginFile. The problem was that calls to the derived class openFile() and closeFile() methods were being done with the asynPortDriver mutex locked. This meant that the driver thread was indeed slowed down by these operations because it was waiting to put the image in the queue. The problem showed up when writing many individual files, e.g. TIFF files, or any file plugin in Single mode. I found it when tracking down a problem with the new ADDexela driver dropping frames at the driver level when writing TIFF files. I also tested the simDetector with 1024x1024 frames at 100 frames/s. If the TIFF plugin was enabled the frame rate dropped to < 40 frames/s. I then fixed NDPluginFile so that it releases the lock during calls to openFile() and closeFile(), and the problems went away. The ADDexela driver no longer drops frames, and the simDetector continues to run at 100 Hz when saving TIFF files. The file plugins now drop files if the queue is not large enough, which is the correct behavior. I have committed the change to ADCore/master and added this to the RELEASE.md: R2-3 (April XXX, 2015)
Mark From: Tom Cobb [[email protected]] At the moment, the NDPluginDriver::driverCallback()<UrlBlockedError.aspx> which is called in the driver thread does a lock() on the plugin before getting a number of parameters to decide whether processCallbacks() should be called and in which thread. If the plugin is busy with the last operation and hasn't unlocked for whatever reason then the driver thread is slowed down. It raises other questions too. Exactly should we be locking for? Are reads of the parameter library atomic? Could we check whether a plugin is in blocking mode without locking up? Could we defer status reporting to the plugin thread if it is currently locked? Thanks, � |
Hang on... Isn't that a "feature"? Derived file writers expect that the lock is taken already, so they can just go ahead and access the parameter library and internal class parameters - at least the HDF5 file writer does that with the assumption that it is safe. It would require some careful modifications to properly support more fine-grained locking/unlocking there. It's not a very difficult or extensive task though. [EDIT: I've create issue #99 to fix the derived file writers to support this change. Please send related comments directly to that issue ticket #99 so not to hijack this one...] |
At the moment, the NDPluginDriver::driverCallback() which is called in the driver thread does a lock() on the plugin before getting a number of parameters to decide whether processCallbacks() should be called and in which thread. If the plugin is busy with the last operation and hasn't unlocked for whatever reason then the driver thread is slowed down.
It raises other questions too. Exactly should we be locking for? Are reads of the parameter library atomic? Could we check whether a plugin is in blocking mode without locking up? Could we defer status reporting to the plugin thread if it is currently locked?
Thanks,
Tom
The text was updated successfully, but these errors were encountered: