Re: Signals in Firefox

2018-11-21 Thread David Teller
Thanks for the suggestions.

Given that they are on an academic deadline and they have already
implemented the feature using straight inotify and a monitor thread, I'd
favor a lesser refactoring with just removing the signals.

Cheers,
 David

On 21/11/2018 22:06, Mike Hommey wrote:
> On Wed, Nov 21, 2018 at 10:22:38AM -0500, Nathan Froyd wrote:
>> On Wed, Nov 21, 2018 at 4:45 AM David Teller  wrote:
>>> What is our policy on using Unix signals on Firefox? I am currently
>>> reviewing a patch by external contributors that involves inotify's
>>> signal API, and I assume it's a bad idea, but I'd like to ask around
>>> first before sending them back to the drawing board.
>>
>> I don't think we have a policy, per se; certainly we already have uses
>> of signals in the JS engine's wasm implementation and the Gecko
>> profiler.  But in those cases, signals are basically the only way to
>> do what we want.  If there were alternative ways to accomplish those
>> tasks besides signals, I think we would have avoided signals.
>>
>> inotify looks like it has a file descriptor-based interface which
>> seems perfectly usable.  Not being familiar with inotify beyond
>> reading http://man7.org/linux/man-pages/man7/inotify.7.html, is there
>> a reason to prefer the signal interface versus the file descriptor
>> interface?  We use the standard gio/gtk event loop, so hooking up the
>> returned file descriptor from inotify_init should not be onerous.
>> widget/gtk/nsAppShell.cpp even contains some code to crib from:
>>
>> https://searchfox.org/mozilla-central/source/widget/gtk/nsAppShell.cpp#275-281
> 
> I'd go one step further. We use Gio from libglib, is there a reason not
> to use the GFileMonitor API, which wraps inotify?
> 
> https://developer.gnome.org/gio/stable/GFileMonitor.html
> 
> Mike
> 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Signals in Firefox

2018-11-21 Thread Mike Hommey
On Wed, Nov 21, 2018 at 10:22:38AM -0500, Nathan Froyd wrote:
> On Wed, Nov 21, 2018 at 4:45 AM David Teller  wrote:
> > What is our policy on using Unix signals on Firefox? I am currently
> > reviewing a patch by external contributors that involves inotify's
> > signal API, and I assume it's a bad idea, but I'd like to ask around
> > first before sending them back to the drawing board.
> 
> I don't think we have a policy, per se; certainly we already have uses
> of signals in the JS engine's wasm implementation and the Gecko
> profiler.  But in those cases, signals are basically the only way to
> do what we want.  If there were alternative ways to accomplish those
> tasks besides signals, I think we would have avoided signals.
> 
> inotify looks like it has a file descriptor-based interface which
> seems perfectly usable.  Not being familiar with inotify beyond
> reading http://man7.org/linux/man-pages/man7/inotify.7.html, is there
> a reason to prefer the signal interface versus the file descriptor
> interface?  We use the standard gio/gtk event loop, so hooking up the
> returned file descriptor from inotify_init should not be onerous.
> widget/gtk/nsAppShell.cpp even contains some code to crib from:
> 
> https://searchfox.org/mozilla-central/source/widget/gtk/nsAppShell.cpp#275-281

I'd go one step further. We use Gio from libglib, is there a reason not
to use the GFileMonitor API, which wraps inotify?

https://developer.gnome.org/gio/stable/GFileMonitor.html

Mike
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: Signals in Firefox

2018-11-21 Thread Nathan Froyd
On Wed, Nov 21, 2018 at 4:45 AM David Teller  wrote:
> What is our policy on using Unix signals on Firefox? I am currently
> reviewing a patch by external contributors that involves inotify's
> signal API, and I assume it's a bad idea, but I'd like to ask around
> first before sending them back to the drawing board.

I don't think we have a policy, per se; certainly we already have uses
of signals in the JS engine's wasm implementation and the Gecko
profiler.  But in those cases, signals are basically the only way to
do what we want.  If there were alternative ways to accomplish those
tasks besides signals, I think we would have avoided signals.

inotify looks like it has a file descriptor-based interface which
seems perfectly usable.  Not being familiar with inotify beyond
reading http://man7.org/linux/man-pages/man7/inotify.7.html, is there
a reason to prefer the signal interface versus the file descriptor
interface?  We use the standard gio/gtk event loop, so hooking up the
returned file descriptor from inotify_init should not be onerous.
widget/gtk/nsAppShell.cpp even contains some code to crib from:

https://searchfox.org/mozilla-central/source/widget/gtk/nsAppShell.cpp#275-281

-Nathan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform