Code review comment for lp:~ted/qtmir/ual-pause

Revision history for this message
Ted Gould (ted) wrote :

On Mon, 2014-10-06 at 12:17 +0000, Gerry Boland wrote:

> As you're removing ProcessController, I suspect you can remove qtmir's
> dependence on process-cpp package.

Not my goal, but it seems so. There were a few pieces left to kill. r262

> +++ src/modules/Unity/Application/taskcontroller.h
> -/*
> - * Copyright (C) 2013-2014 Canonical, Ltd.
> +/* Copyright (C) 2013-2014 Canonical, Ltd.
> Unnecessary change

Fixed r263.

> +++ src/modules/Unity/Application/upstart/applicationcontroller.cpp
> +bool ApplicationController::pauseApplicationWithAppId(const QString& appId)
> + qDebug() << "ApplicationController::pauseApplication FAILED to stop appId=" << appId;
> failed to _pause_ appId
>
> + qDebug() << "ApplicationController::resumeApplication FAILED to stop appId=" << appId;
> failed to _resume_

Fixed r264.

> +++ tests/modules/common/mock_application_controller.h
> + (void) appId;
> Us Qt people tend to use a macro for this: Q_UNUSED(appId);

Fixed r265.

> And couple of higher-level questions about UAL's behaviour (mostly
> confirmations for me):
> 1. UAL changes the OOM-killer score when we pause/resume apps, yes?

Correct.

> 2. UAL gives app a unlikely-to-be-killed OOM score on start of that
> app?

Correct.

> Something that worries me about this change is that apps not managed
> by upstart will no longer have correct OOM-killer weights. Are trust
> helpers managed by upstart these days? Does the OSK get a good
> OOM-killer value? I need to test this carefully.

All processes are managed by Upstart, so they can all have an OOM score
set in their Upstart job. For instance that's how UAL sets the initial
value, it makes Upstart do it on job creation. I don't know that the OSK
job has that configured properly though.

« Back to merge proposal