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 744625 - Search provider should not spawn miners and set the tracker priority
Search provider should not spawn miners and set the tracker priority
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME documents maintainer(s)
GNOME documents maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-02-16 21:06 UTC by Allison Karlitskaya (desrt)
Modified: 2015-03-06 15:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
application: Spawn the miners only when a window is created (1.84 KB, patch)
2015-02-17 08:03 UTC, Debarshi Ray
committed Details | Review
application: Set priority rdf:types only when a window is created (2.44 KB, patch)
2015-02-17 08:30 UTC, Debarshi Ray
committed Details | Review
application: Create the miner proxies asynchronously (4.13 KB, patch)
2015-02-18 09:47 UTC, Debarshi Ray
none Details | Review
application: Create the miner proxies asynchronously (4.38 KB, patch)
2015-02-19 08:37 UTC, Debarshi Ray
committed Details | Review

Description Allison Karlitskaya (desrt) 2015-02-16 21:06:33 UTC
Doing a search in gnome-shell with the gnome-documents search provider enabled results in an unreasonably large number of processes being created, including the gnome-online-miners for gdata, zpj and owncloud.

This is particularly surprising because I don't have accounts configured for any of these things.

I verified that gnome-documents was to blame by disabling all search providers and doing a search (no miners created) and then doing a search with only gnome-documents enabled (lots of miners created).

gnome-photos has a similar bug, which I will file separately.
Comment 1 Debarshi Ray 2015-02-17 08:03:19 UTC
Created attachment 296984 [details] [review]
application: Spawn the miners only when a window is created
Comment 2 Debarshi Ray 2015-02-17 08:30:20 UTC
Created attachment 296987 [details] [review]
application: Set priority rdf:types only when a window is created
Comment 3 Cosimo Cecchi 2015-02-17 19:45:14 UTC
Review of attachment 296984 [details] [review]:

Shouldn't this be done by setting G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START instead?
Comment 4 Cosimo Cecchi 2015-02-17 19:47:04 UTC
Review of attachment 296987 [details] [review]:

Looks good
Comment 5 Debarshi Ray 2015-02-18 01:45:23 UTC
(In reply to Cosimo Cecchi from comment #3)
> Review of attachment 296984 [details] [review] [review]:
> 
> Shouldn't this be done by setting G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START
> instead?

I thought about that, but:

1) The difference between G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START_AT_CONSTRUCTION and G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START was not obvious after reading the documentation, and quickly looking at the code. It appears as if we need G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START_AT_CONSTRUCTION.

2) Gio.DBusProxy.makeProxyWrapper does not let me pass any flags.

3) Reducing the number of things that we do when launching the search provider is probably a good idea any way. Maybe we should use the flag *and* move the proxies out of startup?

I will play with the flags and gnome-photos and see how it works out.
Comment 6 Debarshi Ray 2015-02-18 01:51:21 UTC
(In reply to Debarshi Ray from comment #5)
> (In reply to Cosimo Cecchi from comment #3)
> > Review of attachment 296984 [details] [review] [review] [review]:
> > 
> > Shouldn't this be done by setting G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START
> > instead?
>
> 3) Reducing the number of things that we do when launching the search
> provider is probably a good idea any way. Maybe we should use the flag *and*
> move the proxies out of startup?

Particularly because we are creating these synchronously, which is convenient but useless for the search provider.
Comment 7 Debarshi Ray 2015-02-18 02:13:27 UTC
So, yes, G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START_AT_CONSTRUCTION is what we want. See the patch for gnome-photos in bug 744627 where I did it that way.
Comment 8 Cosimo Cecchi 2015-02-18 03:11:02 UTC
I'm fine with doing both the things you propose, even though we could easily make the construction asynchronous without complicating it too much (gjs will return the GAsyncInitable when async construction is requested IIRC).
I guess we'll have to call new Gio.DBusProxy() manually to set the flag instead of using the GJS convenience... :-(
Comment 9 Debarshi Ray 2015-02-18 09:04:46 UTC
(In reply to Cosimo Cecchi from comment #8)
> I'm fine with doing both the things you propose, even though we could easily
> make the construction asynchronous without complicating it too much (gjs
> will return the GAsyncInitable when async construction is requested IIRC).

The function returned by makeProxyWrapper accepts an asynCallback parameter. The proxy construction becomes asynchronous if that is passed.

Let's make them asynchronous and move them out of startup into window creation.

> I guess we'll have to call new Gio.DBusProxy() manually to set the flag
> instead of using the GJS convenience... :-(

If we have moved them into window creation, then we will end up calling RefreshDB (hence spawning the processes) immediately. Therefore, setting the DO_NOT_AUTO_START_AT_CONSTRUCTION flag does not buy us anything.
Comment 10 Debarshi Ray 2015-02-18 09:47:55 UTC
Created attachment 297081 [details] [review]
application: Create the miner proxies asynchronously
Comment 11 Debarshi Ray 2015-02-19 08:37:38 UTC
Created attachment 297245 [details] [review]
application: Create the miner proxies asynchronously

Avert another possible race.
Comment 12 Debarshi Ray 2015-02-23 18:28:52 UTC
We are entering the freezes. I am pushing them so that we don't miss them for the final release.