Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor configuration logic #82

Closed
armenzg opened this issue Oct 1, 2018 · 3 comments
Closed

Refactor configuration logic #82

armenzg opened this issue Oct 1, 2018 · 3 comments

Comments

@armenzg
Copy link
Contributor

armenzg commented Oct 1, 2018

Currently, we have one single file that wrangles the various platforms we have with the various benchmarks we have.

In some sense, it has a little resemblance to the buildbot-configs configuration files (which is a horrible thought) and I want to refactor it into multiple platform config files (more a-la-TaskCulster-style).

The changes on this issue will require modifications at least in these two places:
https://github.com/mozilla-frontend-infra/js-perf-dashboard/blob/master/src/utils/prepareData.js#L53-L91
https://github.com/mozilla-frontend-infra/js-perf-dashboard/blob/master/src/utils/fetchData.js

Proposal to have these files under src/configuration have:

  • appDefaults.js (e.g. define landing page)
  • desktopDefaults.js
  • androidDefaults.js
  • platform/index.js (e.g. android/index.js)
import DEFAULT_DESKTOP_BENCHMARKS from '../desktopDefaults.js';

const BENCHMARKS = // XXX: Make a deep copy of DEFAULT_DESKTOP_BENCHMARKS

// Make modifications specific to this platform
BENCHMARKS['six-speed'] = {
    compare: {
      'six-speed-sm': {
        color: '#e55525',
        label: 'SpiderMonkey',
        frameworkId: JSBENCH_FRAMEWORK_ID,
        suite: 'six-speed-sm',
        buildType: 'opt',
      },
      'six-speed-v8': {
        color: '#ffcd02',
        label: 'Chrome v8',
        frameworkId: JSBENCH_FRAMEWORK_ID,
        suite: 'six-speed-v8',
        buildType: 'opt',
      },
    },
    labels: ['SpiderMonkey', 'Chrome v8'],
    label: 'Six Speed (JS shell)',
};

const CONFIG = {
      label: 'Linux 64bit',
      platform: 'linux64',
      benchmarks: BENCHMARKS,
};

export default CONFIG;
@klahnakoski
Copy link

  • Must these config files be javascript, or can they be JSON?
  • Are the labels related to the members of the compare?
  • May the data structure be flatter? ie instead of {key: value} we use [{"id":key, "config":value}]?
  • May we flatten the structure even more, assigning a platform property to each benchmark?

I have not even looked a the code yet, so these may be naive questions.

Like this

{
    "label":"Linux64bit",
    "platform":"linux64",
    "benchmarks":[{
        "id":"six-speed",
        "label":"SixSpeed(JSshell)",
        "compare":[
            {
                "id":"six-speed-sm",
                "label":"SpiderMonkey",
                "config":{
                    "color":"#e55525",
                    "label":"SpiderMonkey",
                    "frameworkId":"JSBENCH_FRAMEWORK_ID",
                    "suite":"six-speed-sm",
                    "buildType":"opt"
                }
            },
            {
                "id":"six-speed-v8",
                "label":"Chromev8",
                "config":{
                    "color":"#ffcd02",
                    "label":"Chromev8",
                    "frameworkId":"JSBENCH_FRAMEWORK_ID",
                    "suite":"six-speed-v8",
                    "buildType":"opt"
                }
            }
        ]
    }]
}

or even flatter, at the expense of a bit more redundancy:

[
    {
        "id":"six-speed-sm",
        "label":"SpiderMonkey",
        "benchmark":"six-speed",
        "platform":"linux64",
        "color":"#e55525",
        "frameworkId":"JSBENCH_FRAMEWORK_ID",
        "suite":"six-speed-sm",
        "buildType":"opt"
    },
    {
        "id":"six-speed-v8",
        "label":"Chromev8",
        "benchmark":"six-speed",
        "platform":"linux64",
        "color":"#ffcd02",
        "frameworkId":"JSBENCH_FRAMEWORK_ID",
        "suite":"six-speed-v8",
        "buildType":"opt"
    }
]

@armenzg
Copy link
Contributor Author

armenzg commented Oct 1, 2018

Must these config files be javascript, or can they be JSON?

They don't have to be Javascript.

Are the labels related to the members of the compare?

They're... I will be honest; we could get rid of the top labels. It is only for the purpose of this widget:
image
It is meant to match the colours and labels of the graphs.

May the data structure be flatter? ie instead of {key: value} we use [{"id":key, "config":value}]?

If it doesn't become harder to manipulate programatically or to understand I have no objections.

May we flatten the structure even more, assigning a platform property to each benchmark?

On the most flat version we don't have "label" (text in the dropdown picker) and "platform" (probably also related to the dropdown picker). As long as it is handled somehow I'm fine with that.

@armenzg
Copy link
Contributor Author

armenzg commented Oct 3, 2018

I'm seeing a couple more of issues being filed that would depend on this to be fixed fist. In other words, this has become a bit more important since the Android work they could see it plotted on the fx-health dashboard.

Let me know if you think you could tackle this in the next couple of weeks.

aimenbatool added a commit to aimenbatool/js-perf-dashboard that referenced this issue Oct 16, 2018
aimenbatool added a commit to aimenbatool/js-perf-dashboard that referenced this issue Oct 19, 2018
aimenbatool added a commit to aimenbatool/js-perf-dashboard that referenced this issue Oct 23, 2018
aimenbatool added a commit to aimenbatool/js-perf-dashboard that referenced this issue Oct 23, 2018
aimenbatool added a commit to aimenbatool/js-perf-dashboard that referenced this issue Nov 2, 2018
@armenzg armenzg closed this as completed Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants