Charlie and I just realized that adding a flag could be a security problem because if this is trying to prevent sideloading of executables into chrome in a compromised system then maybe the malware could also change the runtime flags of Chrome. It seems like maybe instead of a runtime flag we should make a compile-time flag for this.

+gab: Is lucky luke is planning on making it possible to schedule work that runs at startup so that it does not affect performance (and therefore wouldn't affect telemetry benchmarks)? Not sure if this is in scope?

There is PostAfterStartupTask() already, otherwise TaskPriority::BACKGROUND tells the scheduler to ideally not run this task while there is other work pending.

For power measurement purposes BACKGROUND doesn't help at all.

What we need is some period of time where Chrome is guaranteed to be relatively idle so that we can measure things like idle context switches, idle CPU usage, typical power usage while playing video, etc. The startup enumeration of modules interferes with this. It was the largest source of CPU time in the browser process which was using the most CPU time of any chrome processes. I'm not saying that this is bad code to run, just that running it at startup for an indeterminate amount of time is causing problems.

Maybe run it with a delay of 30-60 seconds so that we can run our benchmarks quickly before it starts?

What's the point of measuring power on startup though? IMO startup is a bursty operation that should be doing everything it can to utilize machine resources as much as possible for performance? Shouldn't we measure power after startup/quiescence?

Telemetry starts the browser for each benchmark. Is there some good way to know when it has reached quiescence?

In my opinion, the ideal benchmarking picture would look like this:

* Most of Telemetry benchmarks are run with: --disable-unneeded-startup-work. (though if a startup work will affect browser's later perf state, we may want to enable them)

* Telemetry's startup benchmark (which I expect we measure: latency, memory usage & power usage) is not run with the flag. This is to ensure that the total of bursty operations we do on startup is acceptable to the users.

The problem with that is with flagging "startup work" and keeping that up
to date.

Since TaskPriority::BACKGROUND tasks shouldn't be necessary to complete any
visible behavior what we could do is have a flag that tells the
TaskScheduler to not run BACKGROUND tasks at all in this session (until
shutdown as TaskPriority:: BACKGROUND +TaskShutdownBehavior::BLOCK_SHUTDOWN
would still have to eventually run unless you terminate process when
benchmark is done). This would also have the nice benefit to cross-check
that there indeed aren't user visible outcomes that depend on BACKGROUND
tasks.

Le mer. 19 juil. 2017 12:22, nednguyen via monorail <
monorail+v2.3273923485@chromium.org> a écrit :

> What's the point of measuring power on startup though?

We want to measure power shortly after startup because doing otherwise is unwieldy. For example, how long do we have to wait for the module enumeration task to finish? How do we tell when it's finished. And what about other tasks that start after thirty seconds?

> Shouldn't we measure power after startup/quiescence?

Yes, but if it takes 20-60+ seconds to reach quiescence then that makes repeated tests painfully slow. If we had an option to run all startup tasks as quickly as possible that would also work. I don't mind waiting 5-10 seconds for quiescence, I just don't want to have to wait a long period of time.

Ok so having a flag to prevent the TaskScheduler from ever (until shutdown) processing TaskPriority::BACKGROUND work as mentioned in #8 would work?

Eventually all background startup work (and random startup delays: 30s, 60s, etc.) should just be TaskPriority::BACKGROUND tasks in the TaskScheduler so we'll have one place to control them all.

It sounds like that would work, modulo the security concerns around giving bad actors an easy way to disable the module check. Maybe have a flag that delays these background tasks until n-seconds after launch, where n has some maximum value like 500 or so?

Also, from what I understand of the module scanning code it currently doesn't use TaskPriority::BACKGROUND so a flag that delayed these tasks wouldn't help with our number one concern. Here is the scheduling code:

  // Post a delayed task to scan the first module. This forwards directly to
  // ScanImplFinish if there are no modules to scan.
  background_task_runner_->PostDelayedTask(
      FROM_HERE,
      base::Bind(&ModuleEnumerator::ScanImplModule, base::Unretained(this), 0),
      per_module_delay_);

Looking at the code I also see that there is an option to do the scanning as quickly as possible, but I don't see a way to trigger that from the command-line. Is there such a way? That would do for now.

The NextAction date has arrived: 2017-07-21

I just tangentially re-stumbled upon issue 726937 which expresses the same desire. We can look into it when fdoray@ gets back on Monday.

Re. #11: 

a) Max delay 500s SGTM
b) background_task_runner_ is initialized with TaskPriority::BACKGROUND @ [1] (TaskPriority is per TaskRunner, not per task -- except for parallel tasks which have no TaskRunner that is)
c) Scanning as quickly as possible is triggered when navigating to chrome://conflics I believe (pmonette@ would know the details)

[1] https://cs.chromium.org/chromium/src/chrome/browser/win/enumerate_modules_model.cc?type=cs&q=background_task_runner_+base::TaskPriority::BACKGROUND+file:enumerate_modules_model.cc&l=165

(just pinged chrisha@ about this offline)

Regarding comment #11:
The ModuleEnumerator has been completely rewritten as the ModuleDatabase, which we're about to switch to (CL is out for review). It does use the new TaskScheduler, and posts background priority tasks.

Regarding comment #14:
Yup, the "quick as possible" mode is triggered when the work actually blocks UI, so when the user navigates to chrome://conflicts.

Given that the new code does use the task scheduler, should we just add a flag for the task scheduler to delay all background tasks for some period of time? Or would you still prefer a module-enumeration specific flag?

No let's add a task scheduler flag

Le ven. 28 juil. 2017 10:11, chrisha via monorail <
monorail+v2.3389400151@chromium.org> a écrit :

So, just to be clear, who's willing to take this on?

The NextAction date has arrived: 2017-08-01

gab, are you or fdoray willing to add the task scheduler flag?

FWIW, the same security concern exists in that the flag could be abused. We likely want some maximum amount of time that the flag can delay things by.

Finally, the new module enumeration code has just been enabled as an experiment. Barring any weirdness it should become to default in the next couple of weeks.

I'm willing to do the work. I'm tempted to implement as a get-it-over-with flag rather than a defer-it flag. That avoids the security concerns and is simpler in some ways since it means I don't have to worry about being punished if I leave the browser running "too long" before running tests.

Well, get-it-over-with won't work with the scheduler though, there's a constant stream of work and it gets us back to "when should we consider quiescence".

A delay works for me.

It should be easy to hack base/task_scheduler to queue TaskPriority::BACKGROUND tasks for some time after startup (and we can make "startup time" be when the first task is received).

 Issue 842989  has been merged into this issue.