[Draft] Test/shen oc

[pf0n]

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Error

 ⚠️ The pull request body must not be empty.

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26433/head:pull/26433
$ git checkout pull/26433

Update a local copy of the PR:
$ git checkout pull/26433
$ git pull https://git.openjdk.org/jdk.git pull/26433/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26433

View PR using the GUI difftool:
$ git pr show -t 26433

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26433.diff

[bridgekeeper]

👋 Welcome back pf0n! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

⚠️ @pf0n This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

[openjdk]

@pf0n The following labels will be automatically applied to this pull request:

• hotspot
• shenandoah

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[ysramakrishna]

Left a few comments, mostly along the lines of feedback at recent meeting. Feel free to sync up over slack/chime for any follow up/questions (or leave them here if you prefer).

Great start to the changes; I expect once you have the basic thing done here, it'll be a question of doing it (modulo strong vs weak) for the concurrent counting case when no GC as well.

[ysramakrishna]

There are two possible ways of maintaining two implementations, so that you can compare performance as you go.

One is to use compile time switches, such as:

#ifdef NAIVE_IMPLEMENTATION
  naive_code();
#else
  efficient_code();
#endif 

The alternative way is to have it all compile together, but have a JVM option that you enable with -XX:+NaiveImpl which is otherwise false, and you do:

    if (NaiveImpl) {
        naive_code();
   else {
        efficient_code();
   }

A third (probably better) possibility is to just measure and record performance with the naiive implementation, record it, and then throw away the code and do the efficient implementation.

I am fine with whatever approach you take. I'm fine with the third approach as it probably keeps everything clean. The naive implementation was just a stepping stone to the more efficient one, so there's not much point in keeping it after you've gotten rough comparative values. (But do record them to use later in your final presentation / report.)

[ysramakrishna]

Record the performance impact of having this check with the event disabled to measure the impact of the if test.

If it turns out to be substantial, consider templatizing the mark_ref method. (While bearing in mind Knuth's admonition that "premature optimization is the root of all evil" -- so definitely first measure before you change! And when you measure, please record in your log-book/work-book/journal, so you can use it in your report/presentation.)

[ysramakrishna]

Beware the races in updating this table's entries from multiple worker threads.

Consider the two different approaches we discussed:

1. atomic updates, vs
2. thread-local updates followed by a merge of local tables at time of emission.

Consider relative performance of the two approaches especially as we scale: (a) size of the heap (and number of object types/classes) (b) the number of worker threads

[ysramakrishna]

For (1) atomic updates, see class Atomic. I'd imagine this might perform worse.

For (2) thread-local updates followed by merge of local, look at class ParHeapInspectionTask for the general idea. There's definitely a footprint impact. You can allocate the thread local table in the marking loop for each worker thread and one in the heap at the start of marking in the control thread. You then destruct the contents of each as you do currently.

[Optional: I'd also consider a lighter weight object counting event that does not gather any klass histogram data, but records just the total size. Much like Jaroslav's implementation except he was getting it during collection set construction but you could piggyback it onto the marking closure like you do here.]