#470 Enhance Task with Runnable

pending-invalid
Alan Ezust
None
5
2013-06-30
2012-11-17
Thomas Meyer
No

Enhance Task with a Runnable which will be run on the EDT after the task finishes successfully and convert some users of org.gjt.sp.util.AwtRunnableQueue.runAfterIoTasks(Runnable) to use this mechanism instead.

The idea is to get rid of the AwtRunnableQueue in the long term. And to couple the Runnable which should be run after a background task finishes to the background task itself.

Feedback is welcome on this idea.

Discussion

  • Alan Ezust
    Alan Ezust
    2012-12-12

    get rid of AwtRunnableQueue in the long term? It was just introduced. You should mark it deprecated immediately if you want to remove it . Also XSearch will need to be fixed again.

     
  • Thomas Meyer
    Thomas Meyer
    2012-12-28

    Hi Alan,

    Mhh. I guess it would have been better to open this ticket as feature request or something. I just wanted to have the opinion of the other developers on this idea.

    To get rid of the AwtRunnableQueue in the IOTasks classes is easy. But there exists several other users of this "delay work till IO is done" mechanism, which are not so easy to convert. maybe the other users are not possible to convert.

     
  • Alan Ezust
    Alan Ezust
    2012-12-28

    • assigned_to: nobody --> jarekczek
     
  • Alan Ezust
    Alan Ezust
    2012-12-28

    I agree, the AwtRunnableQueue seems like something we really don't need in the API.
    Adding the setRunInDispatch(Runnable) for a Task seems much more elegant.

    Other devs should look at this too since we are trying to finalize an API, but I think it's ok.
    jarek?

    Kazutoshi, Jarek, Matthieu, Dale?

     
  • Alan Ezust
    Alan Ezust
    2013-02-07

    • assigned_to: jarekczek --> kpouer
     
  • I think the reason to have this AwtRunnableQueue is that in the first threadpool some AWT tasks had to be done after all background tasks (the coupling was wrong but it is not easy to take rid of it).
    But if we can do this I have a suggestion:
    In fact what is suggested looks like a lot to SwingWorkers.
    When I wrote the threadpool I didn't use Swingworkers because I didn't find a way to monitor the tasks, but maybe there is a way to it ?

     
  • Alan Ezust
    Alan Ezust
    2013-03-16

    We should decide on what to do about this before releasing 5.1pre1, I think.

     
  • Thomas Meyer
    Thomas Meyer
    2013-04-07

    "When I wrote the threadpool I didn't use Swingworkers because I didn't find
    a way to monitor the tasks, but maybe there is a way to it ?"

    maybe there is, but I didn't look into this yet.

    I would like to close this patch, as I'm working on another idea:

    The idea is to add a TaskManager.addNextTask(currentTask, nextTask) method. Depending on the Task type (subclass IoTask or AwtTask), when the depending task is finshed (i.e. fireDone() is called) the next Task in the next Task List get scheduled in either Background or EDT depending on the task type. The next Task queue is polled and scheduled till the queue is empty or till a AwtTask is hit (The AwtTask is scheduled on the EDT and the remaining Tasks in the next Task queue get's attached to the this AwtTask) I hope to post some code in the next week or so.

    what do you think?

     
  • But in that case it would have the same behavior as we had before, when an AWT task is running, no other task can be started ?

     
  • Thomas Meyer
    Thomas Meyer
    2013-04-13

    So some code, please have a look and tell me what do you think about the direction this is heading!

    "But in that case it would have the same behavior as we had before, when an AWT task is running, no other task can be started ?" -> The current implementation allows several task to run. Or am I missing something?

    This patch is based on the refactor IoTask patch.

     
  • Alan Ezust
    Alan Ezust
    2013-04-13

    so to clarify, do I need to apply 3 patches in this order to test this ticket?
    1. the patch from ticket# 3610167,
    2. the patch below that is for refactoring IO tasks
    3. the Replace-WorkThreadPool-with-executor.patch

    Just tried applying them to trunk, each of them has a couple of hunks that fail, and #3 had 4 hunks that failed.

     
  • Thomas Meyer
    Thomas Meyer
    2013-04-14

    Hi,

    my current patch queue is:
    $ hg qseries
    Fix-Hypersearch-Excessive-Mode-Usage.patch
    HyperSearch-Make-Abort-work-reliable.patch
    TaskManager-Refactor-ioTask.patch
    TaskManager-Refactor-AwtTask-IoTask.patch

    But you should actullally just need these two:
    - TaskManager-Refactor-ioTask.patch (from 3610167)
    - TaskManager-Refactor-AwtTask-IoTask.patch (from this one)

    TaskManager-Refactor-ioTask.patch seems to not apply cleanly on the trunk, because of the other ones in the queue. Shall I rework the patch to apply cleanly? Or can we get the other two applied to the trunk first?

    The third patch is not needed anymore (Replace-WorkThreadPool-with-executor.patch) as it becomes obsolet with the new idea in "TaskManager-Refactor-AwtTask-IoTask.patch". Shall I remove the file from this side?

     
  • Alan Ezust
    Alan Ezust
    2013-04-15

    I don't know what ticket# the Fix-Hypersearch-Excessive-Mode-Usage.patch is in.
    Tried to apply HyperSearch-Make-Abort-work-reliable.patch and failed.
    You ask: "Shall I rework the patch to apply cleanly? Or can we get the other two applied to the trunk first?"
    I would be happy if either or both things happened.

    In general, if your patch depends on another ticket to be applied first, you should list the ticket number there, to help us figure things out faster.

     
  • Alan Ezust
    Alan Ezust
    2013-06-07

    • assigned_to: kpouer --> ezust
     
  • Alan Ezust
    Alan Ezust
    2013-06-09

    • status: open --> open-remind
     
  • Alan Ezust
    Alan Ezust
    2013-06-09

    I can apply the patch cleanly but I get a compile error on buffer.java:307 cannot find symbol getIoTask()

     
  • Alan Ezust
    Alan Ezust
    2013-06-09

    • status: open-remind --> open
     
  • Alan Ezust
    Alan Ezust
    2013-06-09

    Also, does the "Replace WorkThreadPool with Executor Framework.patch" - is that still relevant? If so, then both patches need to be rebased.

     
  • Alan Ezust
    Alan Ezust
    2013-06-30

    Setting as invalid/pending until an updated patch is attached.

     
  • Alan Ezust
    Alan Ezust
    2013-06-30

    • status: open --> pending-invalid