[rvm-research] A bug in GenCopyMutator

[Dingwen Yuan]

Hello All,

I think I found a bug in GenCopyMutator. The original collectionPhase() in this class is:

public void collectionPhase(short phaseId, boolean primary) {
    if (global().traceFullHeap()) {
      if (phaseId == GenCopy.PREPARE) {
        super.collectionPhase(phaseId, primary);
        if (global().gcFullHeap) mature.rebind(GenCopy.toSpace());
        return;
      }
    }

    super.collectionPhase(phaseId, primary);
  }

I think the line "if (global().gcFullHeap) mature.rebind(GenCopy.toSpace());" should be moved to the phase RELEASE. Because the PREPARE of mutator comes before the PREPARE of GenCopy, at that time the toSpace is not yet toggled, so we will premature into the old space which is wrong.

The original pretenure code made the dacapo benchmarks fail(I've tried antlr, fop, pmd). But they can pass with the modified code.

BTW, I worked with version 2.9.2, and I think the bug also exists in later versions.

cheers,

Dingwen

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables unlimited
royalty-free distribution of the report engine for externally facing
server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
Jikesrvm-researchers mailing list
Jikesrvm-researchers@...
https://lists.sourceforge.net/lists/listinfo/jikesrvm-researchers

[Steve Blackburn]

Hi Dingwen,

I don't really understand what you are saying here.

1.  Unless you are doing pretenuring, you will never exercise this  
code path.   Are you doing pretenuring?

2.  The code looks correct to me (it mirrors the code used for the  
collector's allocator which *is* exercised and is correct).

--Steve


On 18/06/2009, at 4:30 PM, Dingwen Yuan wrote:

> Hello All,
>
> I think I found a bug in GenCopyMutator. The original  
> collectionPhase() in this class is:
>
> public void collectionPhase(short phaseId, boolean primary) {
>     if (global().traceFullHeap()) {
>       if (phaseId == GenCopy.PREPARE) {
>         super.collectionPhase(phaseId, primary);
>         if (global().gcFullHeap) mature.rebind(GenCopy.toSpace());
>         return;
>       }
>     }
>
>     super.collectionPhase(phaseId, primary);
>   }
>
> I think the line "if (global().gcFullHeap)  
> mature.rebind(GenCopy.toSpace());" should be moved to the phase  
> RELEASE. Because the PREPARE of mutator comes before the PREPARE of  
> GenCopy, at that time the toSpace is not yet toggled, so we will  
> premature into the old space which is wrong.
>
> The original pretenure code made the dacapo benchmarks fail(I've  
> tried antlr, fop, pmd). But they can pass with the modified code.
>
> BTW, I worked with version 2.9.2, and I think the bug also exists in  
> later versions.
>
> cheers,
>
> Dingwen
> ------------------------------------------------------------------------------
> Crystal Reports - New Free Runtime and 30 Day Trial
> Check out the new simplified licensing option that enables unlimited
> royalty-free distribution of the report engine for externally facing
> server and web deployment.
> http://p.sf.net/sfu/businessobjects_______________________________________________
> Jikesrvm-researchers mailing list
> Jikesrvm-researchers@...
> https://lists.sourceforge.net/lists/listinfo/jikesrvm-researchers

------------------------------------------------------------------------------
Are you an open source citizen? Join us for the Open Source Bridge conference!
Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
Need another reason to go? 24-hour hacker lounge. Register today!
http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
_______________________________________________
Jikesrvm-researchers mailing list
Jikesrvm-researchers@...
https://lists.sourceforge.net/lists/listinfo/jikesrvm-researchers

[Steve Blackburn]

Hi Dingwen,

Daniel took a look too and prodded me off list.  I now see what the  
issue is.   Yes, you're right, we should move it to RELEASE because  
the semi-space flip happens after the collector prepare but before the  
mutator prepare.

I have fixed it in the svn head now, r15715.

See http://jira.codehaus.org/browse/RVM-832

THanks for reporting the issue.

--Steve


On 23/06/2009, at 8:54 PM, Steve Blackburn wrote:

> Hi Dingwen,
>
> I don't really understand what you are saying here.
>
> 1.  Unless you are doing pretenuring, you will never exercise this
> code path.   Are you doing pretenuring?
>
> 2.  The code looks correct to me (it mirrors the code used for the
> collector's allocator which *is* exercised and is correct).
>
> --Steve
>
>
> On 18/06/2009, at 4:30 PM, Dingwen Yuan wrote:
>
>> Hello All,
>>
>> I think I found a bug in GenCopyMutator. The original
>> collectionPhase() in this class is:
>>
>> public void collectionPhase(short phaseId, boolean primary) {
>>    if (global().traceFullHeap()) {
>>      if (phaseId == GenCopy.PREPARE) {
>>        super.collectionPhase(phaseId, primary);
>>        if (global().gcFullHeap) mature.rebind(GenCopy.toSpace());
>>        return;
>>      }
>>    }
>>
>>    super.collectionPhase(phaseId, primary);
>>  }
>>
>> I think the line "if (global().gcFullHeap)
>> mature.rebind(GenCopy.toSpace());" should be moved to the phase
>> RELEASE. Because the PREPARE of mutator comes before the PREPARE of
>> GenCopy, at that time the toSpace is not yet toggled, so we will
>> premature into the old space which is wrong.
>>
>> The original pretenure code made the dacapo benchmarks fail(I've
>> tried antlr, fop, pmd). But they can pass with the modified code.
>>
>> BTW, I worked with version 2.9.2, and I think the bug also exists in
>> later versions.
>>
>> cheers,
>>
>> Dingwen
>> ------------------------------------------------------------------------------
>> Crystal Reports - New Free Runtime and 30 Day Trial
>> Check out the new simplified licensing option that enables unlimited
>> royalty-free distribution of the report engine for externally facing
>> server and web deployment.
>> http://p.sf.net/sfu/businessobjects_______________________________________________
>> Jikesrvm-researchers mailing list
>> Jikesrvm-researchers@...
>> https://lists.sourceforge.net/lists/listinfo/jikesrvm-researchers
>
>
> ------------------------------------------------------------------------------
> Are you an open source citizen? Join us for the Open Source Bridge  
> conference!
> Portland, OR, June 17-19. Two days of sessions, one day of  
> unconference: $250.
> Need another reason to go? 24-hour hacker lounge. Register today!
> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
> _______________________________________________
> Jikesrvm-researchers mailing list
> Jikesrvm-researchers@...
> https://lists.sourceforge.net/lists/listinfo/jikesrvm-researchers

------------------------------------------------------------------------------
Are you an open source citizen? Join us for the Open Source Bridge conference!
Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250.
Need another reason to go? 24-hour hacker lounge. Register today!
http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org
_______________________________________________
Jikesrvm-researchers mailing list
Jikesrvm-researchers@...
https://lists.sourceforge.net/lists/listinfo/jikesrvm-researchers