unity-2d

Merge lp://staging/~dbarth/unity-2d/envvar-jit-flag into lp://staging/unity-2d

  1. envvar-jit-flag
  2. Merge into trunk
Proposed by David Barth
Status: Needs review
Proposed branch: lp://staging/~dbarth/unity-2d/envvar-jit-flag
Merge into: lp://staging/unity-2d
Diff against target: 70 lines (+11/-16)
3 files modified
config.h.in (+0/-6)
data/com.canonical.Unity2d.gschema.xml (+0/-9)
spread/app/spreadview.cpp (+11/-1)
To merge this branch: bzr merge lp://staging/~dbarth/unity-2d/envvar-jit-flag
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Gerry Boland (community) Needs Fixing
Review via email: mp+175480@code.staging.launchpad.net

Commit message

This is a simplified version of the jit-composite feature, using an environment variable in lieu of the dconf binding.

Description of the change

This is a simplified version of the jit-composite feature, using an environment variable in lieu of the dconf binding. Testing showed an issue with using dconf, where the flag is not retrieved properly. Probably due to an early dbus initialization issue with dconf-qt, but since that project has been superseded, it seemed more efficient to fallback to a simpler solution for now.

To post a comment you must log in.
Revision history for this message
David Barth (dbarth) wrote :

To test, use:

UNITY2D_JIT_COMPOSITE=1 ./spread/app/unity-2d-spread

There is a debug statement at the beginning to show which mode the spread runs in.

Reply
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1158
http://s-jenkins:8080/job/unity-2d-ci/36/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=amd64/36
    SUCCESS: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=i386/36

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-2d-ci/36/rebuild

review: Approve (continuous-integration)
Reply
Revision history for this message
Gerry Boland (gerboland) wrote :

If you're not going to use the "jit-composite" dconf key any more, you should remove it from both places in spread/app/spreadview.cpp, and from the schema.

I would also prefer reading UNITY2D_JIT_COMPOSITE in the SpreadView constructor to set a private boolean member of the class, which can be used throughout the lifetime of the spread. Env var can't be changed during runtime, so it avoids useless repeated reads.

qDebug() needed?

review: Needs Fixing
Reply
Revision history for this message
Gerry Boland (gerboland) wrote :

"buggy dconf binding" <- is there a bug for this?

Reply
lp://staging/~dbarth/unity-2d/envvar-jit-flag updated
1159. By David Barth

suppress the dconf key previously added to control the jit flag

1160. By David Barth

read UNITY2D_JIT_COMPOSITE only once

Revision history for this message
David Barth (dbarth) wrote :

Le 07/08/2013 17:45, Gerry Boland a écrit :
> Review: Needs Fixing
>
> If you're not going to use the "jit-composite" dconf key any more, you should remove it from both places in spread/app/spreadview.cpp, and from the schema.
>
> I would also prefer reading UNITY2D_JIT_COMPOSITE in the SpreadView constructor to set a private boolean member of the class, which can be used throughout the lifetime of the spread. Env var can't be changed during runtime, so it avoids useless repeated reads.
>
> qDebug() needed?
 Fixed in the last 2 revisions.

David

Reply
Revision history for this message
David Barth (dbarth) wrote :

Le 07/08/2013 19:30, Gerry Boland a écrit :
> "buggy dconf binding" <- is there a bug for this?
 Nope, the recommendation was just to switch to the newer bindings, which
is why I resorted to using an environment variable in the end. Cost /
opportunity trade-off.

David

Reply
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1160
http://s-jenkins:8080/job/unity-2d-ci/37/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=amd64/37/console
    FAILURE: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=i386/37/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-2d-ci/37/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1160
http://s-jenkins:8080/job/unity-2d-ci/38/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=amd64/38/console
    FAILURE: http://s-jenkins:8080/job/unity-2d-ci/./distribution=precise,flavor=i386/38/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-2d-ci/38/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
Gerry Boland (gerboland) wrote :

Sorry, I forgot about this, bad mail filter meant I missed notifications about this. Please make jenkins happy, then I'll review again.

Reply

Unmerged revisions

1160. By David Barth

read UNITY2D_JIT_COMPOSITE only once

1159. By David Barth

suppress the dconf key previously added to control the jit flag

1158. By David Barth

use a simple environment variable for the jit-composite flag, in lieu of the buggy dconf binding

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches