Android platform scripts

[Braden Shepherdson <br...@chromium.org>]

The Android platform scripts use shelljs.exec's synchronous mode. This is a
terrible hack that leaks filehandles by the hundred, wastes lots of CPU
cycles, and can cause EMFILE on OSX because it runs out of filehandles.

I wanted to rewrite the scripts to be async and use child_process.exec or
.spawn. I started to, but rapidly found that the tangle of callbacks that
resulted was terrible and confusing.

I propose using Q.js here as well (and dropping shelljs as a dependency, if
it was only ever used for .exec).

Any objections?

Braden

Re: Android platform scripts

[Joe Bowser <bo...@gmail.com>]

Do it!

On Fri, Sep 27, 2013 at 8:13 AM, Braden Shepherdson <br...@chromium.org> wrote:
> The Android platform scripts use shelljs.exec's synchronous mode. This is a
> terrible hack that leaks filehandles by the hundred, wastes lots of CPU
> cycles, and can cause EMFILE on OSX because it runs out of filehandles.
>
> I wanted to rewrite the scripts to be async and use child_process.exec or
> .spawn. I started to, but rapidly found that the tangle of callbacks that
> resulted was terrible and confusing.
>
> I propose using Q.js here as well (and dropping shelljs as a dependency, if
> it was only ever used for .exec).
>
> Any objections?
>
> Braden

Re: Android platform scripts

[Braden Shepherdson <br...@chromium.org>]

(Finally back from two days of intern interviews and one day of writing
feedback.)

I would prefer to do it part by part, but this is difficult. If the callers
of the method I port are expecting it to be synchronous, they'll continue
on to the next step too early. If I only change the outermost callers, and
not the callees, there's little benefit to this. Especially since these
have no tests, I'd love to keep it small, but I can't see a way to do that
without porting all of one command, at least. (And there are some
interdependencies.)

Braden


On Fri, Sep 27, 2013 at 5:40 PM, Brian LeRoux <b...@brian.io> wrote:

> Giver. Now, instead if the grand rewrite how about a refactor  of a single
> method for review. Easier for us to buy in and perhaps collab on w you.
> On Sep 27, 2013 7:31 PM, "Braden Shepherdson" <br...@chromium.org> wrote:
>
> > I had since learned that shelljs is used for other things. That's fine,
> > it's not hurting anything unless we use synchronous exec.
> >
> > Braden
> >
> >
> > On Fri, Sep 27, 2013 at 12:47 PM, Andrew Grieve <agrieve@chromium.org
> > >wrote:
> >
> > > SGTM. shelljs is used for other things though, so we won't be able to
> get
> > > rid of it.
> > >
> > >
> > > On Fri, Sep 27, 2013 at 4:13 PM, Braden Shepherdson <
> braden@chromium.org
> > > >wrote:
> > >
> > > > The Android platform scripts use shelljs.exec's synchronous mode.
> This
> > > is a
> > > > terrible hack that leaks filehandles by the hundred, wastes lots of
> CPU
> > > > cycles, and can cause EMFILE on OSX because it runs out of
> filehandles.
> > > >
> > > > I wanted to rewrite the scripts to be async and use
> child_process.exec
> > or
> > > > .spawn. I started to, but rapidly found that the tangle of callbacks
> > that
> > > > resulted was terrible and confusing.
> > > >
> > > > I propose using Q.js here as well (and dropping shelljs as a
> > dependency,
> > > if
> > > > it was only ever used for .exec).
> > > >
> > > > Any objections?
> > > >
> > > > Braden
> > > >
> > >
> >
>

Re: Android platform scripts

[Brian LeRoux <b...@brian.io>]

Giver. Now, instead if the grand rewrite how about a refactor  of a single
method for review. Easier for us to buy in and perhaps collab on w you.
On Sep 27, 2013 7:31 PM, "Braden Shepherdson" <br...@chromium.org> wrote:

> I had since learned that shelljs is used for other things. That's fine,
> it's not hurting anything unless we use synchronous exec.
>
> Braden
>
>
> On Fri, Sep 27, 2013 at 12:47 PM, Andrew Grieve <agrieve@chromium.org
> >wrote:
>
> > SGTM. shelljs is used for other things though, so we won't be able to get
> > rid of it.
> >
> >
> > On Fri, Sep 27, 2013 at 4:13 PM, Braden Shepherdson <braden@chromium.org
> > >wrote:
> >
> > > The Android platform scripts use shelljs.exec's synchronous mode. This
> > is a
> > > terrible hack that leaks filehandles by the hundred, wastes lots of CPU
> > > cycles, and can cause EMFILE on OSX because it runs out of filehandles.
> > >
> > > I wanted to rewrite the scripts to be async and use child_process.exec
> or
> > > .spawn. I started to, but rapidly found that the tangle of callbacks
> that
> > > resulted was terrible and confusing.
> > >
> > > I propose using Q.js here as well (and dropping shelljs as a
> dependency,
> > if
> > > it was only ever used for .exec).
> > >
> > > Any objections?
> > >
> > > Braden
> > >
> >
>

Re: Android platform scripts

[Braden Shepherdson <br...@chromium.org>]

I had since learned that shelljs is used for other things. That's fine,
it's not hurting anything unless we use synchronous exec.

Braden


On Fri, Sep 27, 2013 at 12:47 PM, Andrew Grieve <ag...@chromium.org>wrote:

> SGTM. shelljs is used for other things though, so we won't be able to get
> rid of it.
>
>
> On Fri, Sep 27, 2013 at 4:13 PM, Braden Shepherdson <braden@chromium.org
> >wrote:
>
> > The Android platform scripts use shelljs.exec's synchronous mode. This
> is a
> > terrible hack that leaks filehandles by the hundred, wastes lots of CPU
> > cycles, and can cause EMFILE on OSX because it runs out of filehandles.
> >
> > I wanted to rewrite the scripts to be async and use child_process.exec or
> > .spawn. I started to, but rapidly found that the tangle of callbacks that
> > resulted was terrible and confusing.
> >
> > I propose using Q.js here as well (and dropping shelljs as a dependency,
> if
> > it was only ever used for .exec).
> >
> > Any objections?
> >
> > Braden
> >
>

Re: Android platform scripts

[Andrew Grieve <ag...@chromium.org>]

SGTM. shelljs is used for other things though, so we won't be able to get
rid of it.


On Fri, Sep 27, 2013 at 4:13 PM, Braden Shepherdson <br...@chromium.org>wrote:

> The Android platform scripts use shelljs.exec's synchronous mode. This is a
> terrible hack that leaks filehandles by the hundred, wastes lots of CPU
> cycles, and can cause EMFILE on OSX because it runs out of filehandles.
>
> I wanted to rewrite the scripts to be async and use child_process.exec or
> .spawn. I started to, but rapidly found that the tangle of callbacks that
> resulted was terrible and confusing.
>
> I propose using Q.js here as well (and dropping shelljs as a dependency, if
> it was only ever used for .exec).
>
> Any objections?
>
> Braden
>