Bug 460929

Comment 1 Dani Megert 2015-03-23 09:42:56 EDT

David, any chance we can get this for 4.5?

Comment 2 David Williams 2015-03-23 10:00:04 EDT

(In reply to Dani Megert from comment #1)
> David, any chance we can get this for 4.5?

Yes, there is a chance. (I'd guess-ti-mate 70% chance.)

If you'd like to know the nitty gritty, it's "easy" to "put the data in" with improved labels, but the "UI plugin" that pulls it out needs major work. It is currently written so that it basically reads data out of the data base, makes a bunch of assumptions (all inter-twined in hard to decipher way), and then re-writes the data into a serialized form, and then uses that serialized form as a type of "new" database to base the charts and statistics on. 

I think that whole middle part of "re-writing" the data should be removed, and work directly with the database. I think the original motivation to do that was, apparently, for performance improvements, but it's hard for me to imagine improving much on the performance of a *correctly designed* relational data base? 

But ... I also think any "correct database design" will in part depend on what ever the "current procedure" is for collecting the data (in order to make efficient).

[FYI. I believe some of this is covered in other bugs ... but, I've not bothered to dig that out.]

Comment 3 David Williams 2016-07-16 12:11:23 EDT

May have stumbled upon one source of this problem. A pattern in "find closest" only worked for the years 2003 to 2009! 

http://git.eclipse.org/c/platform/eclipse.platform.releng.git/commit/?id=d173031455240f144616c97b3cf55c7f0d7b26f4

I've tried to fix that "find closest" method but would be surprised that alone would fix this. We'll see. 

So far have only fixed in master.

Comment 4 David Williams 2016-07-27 22:38:50 EDT

Changing the pattern that ended in 2009 didn't seem to make any difference. Might be dead code? 

But, I think I found another spot that is wrong. I hate to commit a change as "trial and error", but since current results are not very helpful, seems the quickest course of action. 

Basically, some code looked for "index of '-' and tried to take rest of "reference" as date, but since the reference build id has two hyphens, then I think it should be lastindexof('-'). 

Plus, from the "input args" the "reference build" is simply the literal build Id of 4.6. I believe in the past, I tried to "codify" the "reference build" as 
the reference build Id suffixed with "-" <timestamp of current build>. 

This may require a change in test collection, as well as test analysis. 

To help debug this, though, I have added a verbose "DEBUG" section in the method. 

Should run the build once with just the changes program changes, to see what values are printed during DEBUG, and then make changes to test collection or test analysis input as appropriate. 

http://git.eclipse.org/c/platform/eclipse.platform.releng.buildtools.git/commit/?id=323604c412c15f59d113ccf66d86bb8b5f3598b9

Comment 5 David Williams 2016-07-28 22:37:05 EDT

(In reply to David Williams from comment #4)

> Should run the build once with just the changes program changes, to see what
> values are printed during DEBUG, and then make changes to test collection or
> test analysis input as appropriate. 
> 
> http://git.eclipse.org/c/platform/eclipse.platform.releng.buildtools.git/
> commit/?id=323604c412c15f59d113ccf66d86bb8b5f3598b9

Perhaps I am mixed up on timing, but I would have thought last night's N-build would have had the "debug printlns" in the log, but is does not: 

https://hudson.eclipse.org/releng/view/Releng/job/ep-collectResults/66/consoleFull

I will, of course, check the next one too (from Thursday night's build). If that one does not have the debug statements, will then try removing the ".dat" files before each analysis (I've always been concerned that "intermediate" representation of the data is what causes problems). 

And, if it is not that then will resort to running locally, and using the debugger. :)

Comment 6 David Williams 2016-07-30 14:45:22 EDT

(In reply to David Williams from comment #5)
> 
> I will, of course, check the next one too (from Thursday night's build). If
> that one does not have the debug statements, will then try removing the
> ".dat" files before each analysis (I've always been concerned that
> "intermediate" representation of the data is what causes problems). 
> 

Since still no sign of DEBUG statements, 

I have tweaked the code to remove pre-existing ".dat" files, if they exists. 

I mistakenly committed with 
http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=8602b67f37d31433e33d0ea6e852cdf957ba5b63

But it is so near 3 PM ("build kick off") I will not revert and do it right. 

The code added was in updateTestResultsPages.sh


  # experiment with deleting previous .dat files, and regenerate all that are needed. 
  # (I believe they are a "performance improvement" for the test analysis itself, but 
  # I suspect they make a lot of assumptions that are no longer true. 
  rm -fr ${ROOT_PERF_DATA}
  RC=$?
  if [[ $RC != 0 ]]
  then
    echo "Could not remove ${ROOT_PERF_DATA}. Return code was $RC. Exiting."
    exit $RC
  fi

Comment 7 David Williams 2016-07-30 15:56:38 EDT

I have also added back the "current build" identifier as a suffix to the "baseline". I did use '-' instead of '_' ... not sure which is correct but given other changes I've made, '-' should be closest to working. 

Was not in time for today's N-build, and, I believe, if we are changing how baselines are coded, in the database, we should start with a fresh database, again, to else there will be several weeks worth of "wrong" baselines and that might cause the heuristic to "not work". 

So, I will try a few local builds before we start stabilization week, just to make sure I haven't made things worse, and if not start with a new database before stabilization builds begin.

Comment 8 David Williams 2016-08-13 10:04:13 EDT

Just to conclude my (probably) final efforts here. I've tried using just the reference build name in the "config" parameter, the reference build name suffixed with '-currentBuildId' and reference build name suffixed with "_currentBuildId". 

In all cases, the only thing that "ends up" visible in the database is the reference build name. No suffixes. So, I am either making a "silly mistake" somewhere or, some place in the "performance test framework" the 'config' values are being modified so that only the reference build name is used and suffixes stripped off? (R-4.6-201606061100 is the reference build for current cases). 

When I talk about "end up" in the database, I come to this conclusion using a "database browser" and looking at "all entries" in each of the tables. 

In the "variations" table, there is only one entry that mentions the baseline, and it says 

|build=R-4.6-201606061100||config=linux.gtk.x86_64||jvm=8.0| 

So, not sure why the "suffixed" versions are not being stored in this table. 

= = = = = = = = = = = = = 

Ah, I did just discover my "silly mistake" and have committed changes today to "try again". 

But the "silly mistake" illustrates an important lesson for Sravan (and others) to be aware of. I had been making the changes in the production overall test.xml file. BUT, in production, we actually run "runTest2.xml" FIRST and save the properties computed to "production.properties" in the root of the testing directory. 

So, in this case, the value of "eclipse.perf.config" was already computed and "read in" by test.xml before test.xml had a chance to set its value, and (as with all Ant properties) once a property is set, it is not changed. 

I doubt if my little fix will solve this bug completely, but thought important to document here the importance of our "runTests2.xml" initialization. 

I don't recall exactly why I went to that model (it was 4 years ago!) but it was related to "order effects" and I found it much easier to compute some properties first, save them as properties used for all tests in that run, and then let test.xml have a simpler time of computing the properties that were not already computed. (We were essentially doing that anyway, with some other property files, and I just expanded its use). I hope this brief documentation helps others in future. 

= = = = = = = = = = = = = = =

Comment 9 David Williams 2016-08-13 20:30:48 EDT

(In reply to David Williams from comment #8)

> Ah, I did just discover my "silly mistake" and have committed changes today
> to "try again". 

Yep. Now using database explorer at least the right data shows up in the variations table: 

|build=R-4.6-201606061100-201608131500||config=linux.gtk.x86_64||jvm=8.0|

It still does not display as expected, but at least (I think) with the data in the table, it is just a matter of correcting some other input and/or some of the display heuristics.

Comment 10 Eclipse Genie 2020-02-17 18:20:38 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.