After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 577772 - Revamp the job scheduling (Banshee.Kernel and UserJob*)
Revamp the job scheduling (Banshee.Kernel and UserJob*)
Product: banshee
Classification: Other
Component: general
git master
Other All
: High enhancement
: 1.x
Assigned To: Banshee Maintainers
Banshee Maintainers
Depends on:
Reported: 2009-04-02 20:40 UTC by Gabriel Burt
Modified: 2020-03-17 08:26 UTC
See Also:
GNOME target: ---
GNOME version: ---

move to hyena jobs (17.86 KB, patch)
2010-11-07 15:06 UTC, olivier dufour
needs-work Details | Review
move metadata service job to hyena jobs (18.98 KB, patch)
2010-11-09 19:04 UTC, olivier dufour
reviewed Details | Review
patch revamp metadata jobs (39.97 KB, patch)
2010-11-29 23:21 UTC, olivier dufour
none Details | Review
patch revamp metadata jobs (48.59 KB, patch)
2010-11-29 23:25 UTC, olivier dufour
none Details | Review
patch to simplify metadata service (83.02 KB, patch)
2010-11-30 18:47 UTC, olivier dufour
none Details | Review
patch revamp metadata jobs 2 (86.15 KB, patch)
2010-12-09 07:16 UTC, olivier dufour
needs-work Details | Review

Description Gabriel Burt 2009-04-02 20:40:01 UTC
The existing Banshee.Kernel and UserJob* code should be unified and improved.  IF possible, all jobs should use the db for their state - eg for a write-metadata-to-file job, it would:

1) keep track of when it last saved data to disk
2) select a track that where CoreTracks.UpdatedAt > last-write-time and write it out
3) repeat 2 until no more

This gives us the huge advantage of being able to restart on the next run such tasks if the user quits before we're done.

Also, we could have some smart scheduling to improve performance.

I've got some code for this started in the jobs branch of
Comment 1 Gabriel Burt 2009-05-12 00:01:19 UTC
The new Hyena.Jobs code is in git master, and many jobs are changed over to using it, but not all - Banshee.Kernel is still around/in use.
Comment 2 olivier dufour 2010-11-07 15:06:22 UTC
Created attachment 173991 [details] [review]
move to hyena jobs

here is a patch to use hyena jobs.
If all work Banshee.Kernel can be removed.
I have test with podcasts but need new sound to get new covert art.
Comment 3 Gabriel Burt 2010-11-07 21:17:48 UTC
Review of attachment 173991 [details] [review]:

Banshee.Kernel would execute jobs serially on a thread, where Hyena.Jobs.Scheduler will attempt to run jobs in parallel, and does no threading work itself.  Whatever thread schedule.Add is called from is the thread the jobs will run on, unless that jobs does its own threading or inherits from SimpleAsyncJob.  We should probably add a "Network" resource to these jobs to ensure they run serially, and should probably have MetadataServiceJob inherit from SimpleAsyncJob.

::: src/Core/Banshee.Services/Banshee.Metadata/MetadataService.cs
@@ +157,2 @@
+            Job job = sender as Job;

Should stop listening to job.Finished here

::: src/Core/Banshee.Services/Banshee.Metadata/MetadataServiceJob.cs
@@ +78,1 @@

Should add a call to Hyena.ThreadAssist.AssertNotInMainThread () here and then test to make sure this isn't getting called from the main/GUI thread.  Banshee.Kernel executed its items serially on a different thread, but Hyena.Jobs doesn't do that automatically for you -- jobs have to do their own threading (and/or inherit from SimpleAsyncJob).
Comment 4 olivier dufour 2010-11-09 19:04:51 UTC
Created attachment 174153 [details] [review]
move metadata service job to hyena jobs

OK, I have done a new patch with your feedback.
I do not set resource Network for FileSystemQueryJob by not calling base() ctor whereas I call it on all other metadataServiceJob inherited class.
is it ok or FileSystemQueryJob need to be with resource too?
Comment 5 Gabriel Burt 2010-11-17 21:02:21 UTC
Review of attachment 174153 [details] [review]:

Thanks Olivier, looking better.  I wish a few things were different:

1) We use the normal job scheduler (ServiceManager.JobScheduler)
2) The only job we add to the scheduler is a MetadataJob : SimpleAsyncJob, that in its Run method loops through the various service/job classes serially.  The service/jobs should inherit from some simple class or interface, not be Hyena.Jobs, etc.

Sorry, not trying to dump on you.  Just trying to brain-dump why I'm hesitant to accept this patch (or work on this part of the code at all -- it's way too complex for what it does).
Comment 6 olivier dufour 2010-11-29 23:21:57 UTC
Created attachment 175505 [details] [review]
patch revamp metadata jobs

Ok, this time, I have done a big refactoring to simplify chain of inheritance...
I have cut the job class in 2. the LookupJob and the scheduller job.
Hope you will enjoy design and simplification for maintainability.
Can you review it ? It will need a lot of test to not break or add regression.
Comment 7 olivier dufour 2010-11-29 23:25:39 UTC
Created attachment 175506 [details] [review]
patch revamp metadata jobs

oops, wrong file (forget to add some files)
Comment 8 olivier dufour 2010-11-30 11:17:50 UTC
To simplify, I think to merge lookupjob into provider.
So we will end with:
Service just add regular metadataServiceJob on scheduler which will loop on provider to get tags and cover.

I will rename List<StreamTag> Run () to "Process ()" to avoid confusing with job Run method and IMetadataLookupJob will be merge with IMetadataProvider.

Are you OK with design ?

public interface IMetadataProvider
    IBasicTrackInfo Track { get; }
    IEnumerable<StreamTag> Process ();
    void Cancel (IBasicTrackInfo track);
    void CancelAll ();
Comment 9 olivier dufour 2010-11-30 11:20:14 UTC
Hmm cancel must be moved to Service and removed from MetadataProvider too...
So interface become:
public interface IMetadataProvider
    IBasicTrackInfo Track { get; }
    IEnumerable<StreamTag> Process ();
Comment 10 olivier dufour 2010-11-30 18:47:46 UTC
Created attachment 175566 [details] [review]
patch to simplify metadata service

Ok, I have finished my refactoring idea.
Can you take a look, it is far better...
I have remove the job mixed up things and even remove the need of couple provider/lookupJob to just need of provider which contain all.
It is not tested but it compile and it is just to have feedback to know if go in the good direction...before debuging/testing
Comment 11 Gabriel Burt 2010-12-08 21:06:36 UTC
Olivier, looks like you forgot to `git add` PodcastImageMetadataProvider.cs
Comment 12 olivier dufour 2010-12-09 07:16:17 UTC
Created attachment 176112 [details] [review]
patch revamp metadata jobs 2

here I add the missing file...
Comment 13 Kevin Anthony 2011-10-09 20:00:05 UTC
Review of attachment 176112 [details] [review]:

Does not apply on banshee 2.2 master:

git apply 0001-Metadata-Jobs-Simplify-Job-system-and-use-regular-sc.patch
0001-Metadata-Jobs-Simplify-Job-system-and-use-regular-sc.patch:1329: trailing whitespace.
0001-Metadata-Jobs-Simplify-Job-system-and-use-regular-sc.patch:1331: trailing whitespace.
0001-Metadata-Jobs-Simplify-Job-system-and-use-regular-sc.patch:1358: trailing whitespace.
0001-Metadata-Jobs-Simplify-Job-system-and-use-regular-sc.patch:1359: trailing whitespace.
// Copyright 2010 
0001-Metadata-Jobs-Simplify-Job-system-and-use-regular-sc.patch:1360: trailing whitespace.
error: patch failed: src/Core/Banshee.Services/Banshee.Metadata.MusicBrainz/MusicBrainzQueryJob.cs:1
error: src/Core/Banshee.Services/Banshee.Metadata.MusicBrainz/MusicBrainzQueryJob.cs: patch does not apply
error: patch failed: src/Core/Banshee.Services/Banshee.Metadata.Rhapsody/RhapsodyQueryJob.cs:61
error: src/Core/Banshee.Services/Banshee.Metadata.Rhapsody/RhapsodyQueryJob.cs: patch does not apply
error: patch failed: src/Core/Banshee.Services/Banshee.Services.csproj:70
error: src/Core/Banshee.Services/Banshee.Services.csproj: patch does not apply
error: patch failed: src/Extensions/Banshee.Podcasting/Banshee.Podcasting.csproj:120
error: src/Extensions/Banshee.Podcasting/Banshee.Podcasting.csproj: patch does not apply
Comment 14 André Klapper 2020-03-17 08:26:58 UTC
Banshee is not under active development anymore and had its last code changes more than three years ago. Its codebase has been archived.

Closing this report as WONTFIX as part of Bugzilla Housekeeping to reflect
reality. Please feel free to reopen this ticket (or rather transfer the project
to GNOME Gitlab, as GNOME Bugzilla is being shut down) if anyone takes the
responsibility for active development again.
See for more info.