RFR: 8149793: DirtyCardQueueSet::apply_closure_to_completed_buffer_helper isn't helpful

[Kim Barrett]

> On Feb 16, 2016, at 4:07 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi Kim,
> 
>  some minor comments about the change:
> 
> On Mo, 2016-02-15 at 02:24 -0500, Kim Barrett wrote:
>> Please review this small cleanup, merging a non-helpful "helper" into
>> its only caller.
>> 
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8149793
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~kbarrett/8149793/webrev.00/
>> 
> 
>  - could the assert be enhanced to print the values of the checked
> variables to see what actually failed?

I’m not sure what change you would like.  The assert fails when during_pause
is true and stop_at is non-zero.  The specific non-zero value doesn’t seem very
interesting to me here.

>  - I would prefer if in the if-clause, the seemingly more common case
> of the buffer should be put first, i.e.
> 
> BufferNode* nd = ...;
> if (nd != NULL) {
>  // work on buffer
> } else {
>  return false;
> }
> 
> like it has been in the original code.

I like it better with the code for the two cases are both close to the test, rather than
a big blob of “then” code between the test and the “else”.  Also, a later cleanup
in my queue changes it to

  if (nd == NULL) {
    …;
  } else if (apply_closure_to_buffer(cl, nd, ...) {
    …;
  } else {
    enqueue_complete_buffer(nd);
    return false;
  }