van-smith / OPBM

Open Productivity Benchmark
1 stars 0 forks source link

Separate open, run and close scripts do not work with harness design #69

Closed ghost closed 13 years ago

ghost commented 13 years ago

Per the note on Sep 09 commit (https://github.com/van-smith/OPBM/commit/4c212ec3ab588ef584d8f81f2ef1143fd6d2b851), and email mentioning the same, the new scripts being submitted do not work with the harness design of installing and restoring user registry key settings before and after a run.

Background / Info:

The harness supports a feature to install Office2010 registry keys before a run, and uninstall them after a run. The script plugin (opbm.dll) is designed to do this at the start of an executable, and remove them at the end of an executable (as one of the last things it does before terminating), as is in the Alice and Heat scripts and the functions Office2010SaveRegistryKeys(), Office2010InstallRegistryKeys() and Office2010RestoreRegistryKeys() in opbmCommon.au3.

The new script designs being pushed run three separate executables per instance test, and do not operate in this way. They must be changed to either:

1) Install and uninstall the registry keys at the start and end of each executable, or 2) Be rolled into a single executable with save, install and restore at the start and end.

(#2 is the preferred method because OPBM uses include files for all common functions. Because of this inclusion, there is no need to have separate "OpenWord" and "CloseWord" scripts, when we can call openWord() and closeWord() functions from within a single run script).

If this is not done, this feature of the harness will not be implemented, and failures will occur on systems which require macros to be allowed via registry key settings, for example, because those registry keys are not being installed or uninstalled properly.

Note: An Oct 31 target change to the harness (#54 cleanup) will overcome this limitation by addressing it more richly and completely, but until then the design is to install keys at the start of a script, uninstall at the end of a script, as per Alice and Heat.

van-smith commented 13 years ago

I do not recall any email where you asked me if invalidating existing scripts was an acceptable solution to this problem. I discussed at length with you why I created scripts like this.

Set all of the registry keys before and after each run instead of before and after each atom. At the moment, the most important thing is to be able to make successful trial runs and official runs.

ghost commented 13 years ago

I don't understand your comment "invalidating existing scripts". The scripts that are causing this issue are new ones.

van-smith commented 13 years ago

Rick, I quote you:

"This fix will not work with the existing publisher, access or powerpoint scripts because they use separate executables for open, running the test, and closing."

The scripts are working as designed. Your change broke them (apparently).

Just move the registry save/restore operations at the beginning and ending of each run like I have already suggested to do (and had assumed you were doing).

ghost commented 13 years ago

Oh. No, the comment I made was with the new scripts that were just added, and were existing as of the time of my message. There was no new change after your scripts were uploaded. The design had been there from the beginning. And all of the new scripts added in this three-component way would not work.

I will move the registry code.

ghost commented 13 years ago

To be completely clear: The new fix I added extended previous abilities which already existed and were part of the Office2010 registry key support design. My comment was that the new scripts that had been added would have no impact by these additions because they weren't using those existing function calls, and were of a completely different design than the initial Alice and Heat Office2010 scripts.

ghost commented 13 years ago

This change will make a portion of the scripts now dependent upon the harness. I believe one of your primary driving reasons for including scoring inside the scripts, rather than inside the harness, was that the scripts be able to be run autonomously, completely outside of the harness. The existing procedure implements that facet of overall design. This new change will undo it.

ghost commented 13 years ago

Regarding this change, there is one other aspect that needs to be discussed. When you get a chance, please call me.

van-smith commented 13 years ago

Atoms. Think Atoms. You seem to want to roll more and more functionality into each unit.

The registry setting could be placed in atoms, although I am not suggesting that at the moment.

I've mentioned to you many times about the registry settings being imposed at the beginning and ending of each run.

van-smith commented 13 years ago

You wrote "Regarding this change, there is one other aspect that needs to be discussed. When you get a chance, please call me."

Please document this either through email or here.

ghost commented 13 years ago

"Atoms. Think Atoms. You seem to want to roll more and more functionality into each unit." The only template I had for script design was Alice and Heat. They were individual scripts with multiple timing events within. I had assumed that was the design standard and built the harness around it. I had also assumed the recent open/run/close scripts were either mistakes, or interim solutions that would be more properly addressed later. And I do not recall any discussions about having three-fold scripts for open, run and close. That's not to say that there weren't any, but I am not recalling any off the top of my head.

ghost commented 13 years ago

"Please document this either through email or here."

The harness records timing events for constructed event sequences, meaning when a run is requested the harness builds manifest.xml, which contains the header portion of opbm.benchmarks.manifest.run entries, and each of these run entries serves as an anchor point for processing, as well as a reference back to the original scripts.xml from which it was built. This construction allows a full follow-through from scripts.xml to final run scores.

Each run identifies every operation taking place in the benchmark. For official runs there are three identical run entries in sequence, but with unique uuid values for abstract items (see next paragraph). For trial runs or a single instance runs being executed (atom, molecule, scenario, suite), then one run entry exists with fully unique uuid entries.

Within each run, every abstract item (an abstract item is a sequence item within each atom) is recorded and uniquely tagged by a uuid field, which is referred to later as the manifestworkletuuid throughout the rest of the manifest.xml file, along with an atomuuid field, which is uniquely assigned to every atom when the script is built and serves to identify the physical atom in scripts.xml this worklet came from.

The atomuuid is unique per atom, but is not unique in manifest.xml, as it is repeated for every entry that uses it in the constructed script.

The harness uses this relationship to correlate scores, averages, etc. For multiple runs, entries from run1 can be correlated with run2, and with run3, with their atoms being correlated together. The harness uses this relationship to roll values together, to relate each atom from each run into cumulative scores in the opbm.resultsdata.aggregate.byAtom section, which then later feeds into the min/max/geo/avg/cv values which are populated on every manifestworkletuuid entry for each run.

By employing an open/run/close design methodology, in cases where more than one run test employs the same open/close atoms, then those atoms will be referenced more times than there are runs (if two workloads use it, then an official run will see six instances). It will affect the number of instances showing up, and will affect the averages by not only its own workload, but also the overhead of the other as well.

Rather than a particular workload (like Alice, or Earthquake) having averages across only its own run times and events, it will also include open/close run times/timing events from all of the other scripts that employed those atoms.

While this is a by-design feature (that all atoms should be rolled up in this way), I believe it is not a feature that's in line with your assumed behavior--based on the introduction of these new scripts all utilizing this new design. And being as we have not discussed these matters previously, I mention it here as an issue.

The harness was designed to record and correlate timing events within each atom. It was designed to roll those timing event scores to an atom score, and roll atom scores up to a molecule score, and roll molecule scores up to a scenario score, and roll scenario scores up to a suite score, and roll suite scores up to a total score, and to do the same with times. This has been its design and operation since conception. It was based on the operation of Alice and Heat, and every other script that has been created up until the recent additions made on August 31, and those additional ones uploaded today.

van-smith commented 13 years ago

This appears to be satisfactorily fixed. Runs have completed on three SUTs including an Official Run on Llano (sans Chrome installation).