sablevm-developer

[Sablevm-developer] branch permissions and tag creation

[Chris P. <chr...@ma...>]

Hi,

As an experiment, I am preparing a tag for Greg or David to
merge into staging.  It will contain a compile-time option to enable
inflating all contended thin locks (currently this is not done; see back
in the list about how in some situations it might give you a small 
benefit).  The option is called --enable-full-lock-inflation.

I did:

$ svn cp svn+ssh://svn.sablevm.org/public/sablevm/branches/staging
svn+ssh://svn.sablevm.org/public/developer/chris/tags/inflate_thin_locks
$ cd
$ svn co 
svn+ssh://svn.sablevm.org/public/developer/chris/tags/inflate_thin_locks
$ cd inflate_thin_locks/
$ svn merge -r 2132:2143 
svn+ssh://svn.sablevm.org/public/developer/chris/sandbox/sablevm
[ fix merge ]
$ svn commit
[ enter log message ]
Sending        configure.ac
Sending        doc/sablevm.1.in
Sending        src/libsablevm/thread.c
Transmitting file data ...subversion/libsvn_client/commit.c:677: 
(apr_err=165001)
svn: Commit failed (details follow):
subversion/libsvn_repos/hooks.c:101: (apr_err=165001)
svn: 'pre-commit' hook failed with error output:
error: you don't have enough permissions for this transaction:
you can't update developers/chris/tags/inflate_thin_locks/configure.ac
you can't update developers/chris/tags/inflate_thin_locks/doc/sablevm.1.in
you can't update 
developers/chris/tags/inflate_thin_locks/src/libsablevm/thread.c

subversion/clients/cmdline/util.c:349: (apr_err=165001)
svn: Your commit message was left in a temporary file:
subversion/clients/cmdline/util.c:349: (apr_err=165001)
svn:    '/home/research/ccl/cpicke/inflate_thin_locks/svn-commit.tmp'

Is there some way for me to fix this?  Am I doing this the wrong way?  I 
looked in the documentation and in 'svn help' but couldn't see anything. 
  What's strange is that I was able to create the branch in the first 
place without any problems.  I am using a 0.37 client.

Cheers,
Chris

Re: [Sablevm-developer] branch permissions and tag creation

[Etienne G. <gag...@uq...>]

Hi Chris,

You can't modify a tag.  A tag is a "snapshot".

You should make the changes in your sandbox, e.g.


svn cp svn+ssh://svn.sablevm.org/public/sablevm/branches/staging \
svn+ssh://svn.sablevm.org/public/developer/chris/sandbox/inflate_thin_locks

svn co ...
svn merge ...
svn ci

Then,

svn cp
svn+ssh://svn.sablevm.org/public/developer/chris/sandbox/inflate_thin_locks
svn+ssh://svn.sablevm.org/public/developer/chris/tags/inflate_thin_locks

You can create (svn cp) and delete (svn rm) tags, but you can't modify them.

Etienne

Chris Pickett wrote:
> Hi,
> 
> As an experiment, I am preparing a tag for Greg or David to
> merge into staging.  It will contain a compile-time option to enable
> inflating all contended thin locks (currently this is not done; see back
> in the list about how in some situations it might give you a small 
> benefit).  The option is called --enable-full-lock-inflation.
> 
> I did:
> 
> $ svn cp svn+ssh://svn.sablevm.org/public/sablevm/branches/staging
> svn+ssh://svn.sablevm.org/public/developer/chris/tags/inflate_thin_locks
> $ cd
> $ svn co 
> svn+ssh://svn.sablevm.org/public/developer/chris/tags/inflate_thin_locks
> $ cd inflate_thin_locks/
> $ svn merge -r 2132:2143 
> svn+ssh://svn.sablevm.org/public/developer/chris/sandbox/sablevm
> [ fix merge ]
> $ svn commit
> [ enter log message ]
> Sending        configure.ac
> Sending        doc/sablevm.1.in
> Sending        src/libsablevm/thread.c
> Transmitting file data ...subversion/libsvn_client/commit.c:677: 
> (apr_err=165001)
> svn: Commit failed (details follow):
> subversion/libsvn_repos/hooks.c:101: (apr_err=165001)
> svn: 'pre-commit' hook failed with error output:
> error: you don't have enough permissions for this transaction:
> you can't update developers/chris/tags/inflate_thin_locks/configure.ac
> you can't update developers/chris/tags/inflate_thin_locks/doc/sablevm.1.in
> you can't update 
> developers/chris/tags/inflate_thin_locks/src/libsablevm/thread.c
> 
> subversion/clients/cmdline/util.c:349: (apr_err=165001)
> svn: Your commit message was left in a temporary file:
> subversion/clients/cmdline/util.c:349: (apr_err=165001)
> svn:    '/home/research/ccl/cpicke/inflate_thin_locks/svn-commit.tmp'
> 
> Is there some way for me to fix this?  Am I doing this the wrong way?  I 
> looked in the documentation and in 'svn help' but couldn't see anything. 
>  What's strange is that I was able to create the branch in the first 
> place without any problems.  I am using a 0.37 client.
> 
> Cheers,
> Chris
> 
> 
> -------------------------------------------------------
> This SF.Net email is sponsored by: Oracle 10g
> Get certified on the hottest thing ever to hit the market... Oracle 10g. 
> Take an Oracle 10g class now, and we'll give you the exam FREE. 
> http://ads.osdn.com/?ad_id=3149&alloc_id=8166&op=click
> _______________________________________________
> Sablevm-developer mailing list
> Sab...@li...
> https://lists.sourceforge.net/lists/listinfo/sablevm-developer
> 
> 

-- 
Etienne M. Gagnon, Ph.D.             http://www.info.uqam.ca/~egagnon/
SableVM:                                       http://www.sablevm.org/
SableCC:                                       http://www.sablecc.org/

Re: [Sablevm-developer] branch permissions and tag creation

[Chris P. <chr...@ma...>]

Hi Etienne,

Okay, thanks.  So just two more quick questions:

1) Under the new merge policy, is it acceptable to create the 
inflate_thin_locks/ branch in my sandbox, copy the merged changes from 
the working copy of the tag I just created, and then commit that with a 
"Merged ..." message?

2) I merged '-r 2132:2143 sandbox/sablevm/' into my 
tags/inflate_thin_locks/ working copy.  During the merge, I noticed 
there were some very small changes or comments I needed to add.  I added 
the comments to my main sandbox/sablevm/ branch as well as the 
tags/inflate_thin_locks/ branch, and committed the changes in 
sandbox/sablevm/ as r2144.

I was planning to write something like

"Merged with command:
svn -r 2132:2144 ..."

as my log message.  Is that sort of thing okay?  Or would I need to do 
two merges in this case?

Cheers,
Chris

Etienne Gagnon wrote:
> Hi Chris,
> 
> You can't modify a tag.  A tag is a "snapshot".
> 
> You should make the changes in your sandbox, e.g.
> 
> 
> svn cp svn+ssh://svn.sablevm.org/public/sablevm/branches/staging \
> svn+ssh://svn.sablevm.org/public/developer/chris/sandbox/inflate_thin_locks
> 
> svn co ...
> svn merge ...
> svn ci
> 
> Then,
> 
> svn cp
> svn+ssh://svn.sablevm.org/public/developer/chris/sandbox/inflate_thin_locks
> svn+ssh://svn.sablevm.org/public/developer/chris/tags/inflate_thin_locks
> 
> You can create (svn cp) and delete (svn rm) tags, but you can't modify them.
> 
> Etienne
> 

Re: [Sablevm-developer] branch permissions and tag creation

[Chris P. <chr...@ma...>]

Chris Pickett wrote:
> 
> I was planning to write something like
> 
> "Merged with command:
> svn -r 2132:2144 ..."
   ^^^^^^^^^^^^^^^^^^^^^
of course i meant to type "svn merge -r 2132:2144 ..." here.

Re: [Sablevm-developer] branch permissions and tag creation

[Grzegorz B. P. <ga...@de...>]

On (30/04/04 17:34), Chris Pickett wrote:
> Hi Etienne,
> 
> Okay, thanks.  So just two more quick questions:
> 
> 1) Under the new merge policy, is it acceptable to create the 
> inflate_thin_locks/ branch in my sandbox, copy the merged changes from 
> the working copy of the tag I just created, and then commit that with a 
> "Merged ..." message?

Well, assuming the result would be just the same, as if you did the
merge in the right place - then sure. But why not do it the right
way, in the right place in the first place?

> 2) I merged '-r 2132:2143 sandbox/sablevm/' into my 
> tags/inflate_thin_locks/ working copy.  During the merge, I noticed 
> there were some very small changes or comments I needed to add.  I added 
> the comments to my main sandbox/sablevm/ branch as well as the 
> tags/inflate_thin_locks/ branch, and committed the changes in 
> sandbox/sablevm/ as r2144.
> 
> I was planning to write something like
> 
> "Merged with command:
> svn -r 2132:2144 ..."
> 
> as my log message.  Is that sort of thing okay?  Or would I need to do 
> two merges in this case?

The merge policy is getting especially important when merging things
into staging and further.  There's nothing that prevents you from being
inacurrate and messy in your own sandbox.  If you think that this
information is good enough for you - then, I think it's fine.

The reason why merge policy forbids modifications not strictly necessary
to the merge itself is, as I read it, that if the merge gets reverted
for any reason - these changes are lost.  The other reason is that if
you want to repeat the merge in another place/time, you know exactly
what was merged by looking at the log message.

In short: it's just much better and easier to abide by the policy ;-)

Cheers,

				GBP

-- 
Grzegorz B. Prokopski <ga...@de...>
Debian GNU/Linux      http://www.debian.org
SableVM - LGPLed JVM  http://www.sablevm.org
Why SableVM ?!?       http://devel.sablevm.org/wiki/WhySableVM

Re: [Sablevm-developer] branch permissions and tag creation

[Chris P. <chr...@ma...>]

Grzegorz B. Prokopski wrote:

>>1) Under the new merge policy, is it acceptable to create the 
>>inflate_thin_locks/ branch in my sandbox, copy the merged changes from 
>>the working copy of the tag I just created, and then commit that with a 
>>"Merged ..." message?
> 
> 
> Well, assuming the result would be just the same, as if you did the
> merge in the right place - then sure. But why not do it the right
> way, in the right place in the first place?

[...]

> The merge policy is getting especially important when merging things
> into staging and further.  There's nothing that prevents you from being
> inacurrate and messy in your own sandbox.  If you think that this
> information is good enough for you - then, I think it's fine.
> 
> The reason why merge policy forbids modifications not strictly necessary
> to the merge itself is, as I read it, that if the merge gets reverted
> for any reason - these changes are lost.  The other reason is that if
> you want to repeat the merge in another place/time, you know exactly
> what was merged by looking at the log message.
> 
> In short: it's just much better and easier to abide by the policy ;-)

Thank-you for the clarification.  I just wanted make sure I didn't go 
and do something terrible.

Anyway, the tag is there now.

Cheers,
Chris

Re: [Sablevm-developer] branch permissions and tag creation

[Grzegorz B. P. <ga...@de...>]

On (30/04/04 16:22), Chris Pickett wrote:
> Hi,
> 
> As an experiment, I am preparing a tag for Greg or David to
> merge into staging.  It will contain a compile-time option to enable
> inflating all contended thin locks (currently this is not done; see back
> in the list about how in some situations it might give you a small 
> benefit).  The option is called --enable-full-lock-inflation.

Firstly, how do I get the diff of your changes? I tried: 

svn diff
svn+ssh://svn.sablevm.org/public/developers/chris/tags/inflate_thin_locks
-r 2146 svn+ssh://svn.sablevm.org/public/sablevm/branches/staging |less

to compare your tag to the last revision of sablevm-staging you've
merged but it seems to be showing lots of other stuff too, like
lt_dlopen stuff Etienne merged lately from Melanie's sandbox,
and I can't seem to find *your* changes.

Secondly, has it been agreed that this stuff is to be merged into
staging? I'd like to hear from Etienne before we merge.
(honestly, I wish I could see the diff before asking ;-)

Cheers,

					GBP

-- 
Grzegorz B. Prokopski <ga...@de...>
Debian GNU/Linux      http://www.debian.org
SableVM - LGPLed JVM  http://www.sablevm.org
Why SableVM ?!?       http://devel.sablevm.org/wiki/WhySableVM

Re: [Sablevm-developer] branch permissions and tag creation

[Etienne G. <gag...@uq...>]

Hi,

Grzegorz B. Prokopski wrote:
> Secondly, has it been agreed that this stuff is to be merged into
> staging? I'd like to hear from Etienne before we merge.
> (honestly, I wish I could see the diff before asking ;-)

I'd like to see a comparison of this proposal with the elimination of 
fat locks counter proposal, on performance, before we put this code (or 
the counter proposal) in staging.

Etienne

-- 
Etienne M. Gagnon, Ph.D.             http://www.info.uqam.ca/~egagnon/
SableVM:                                       http://www.sablevm.org/
SableCC:                                       http://www.sablecc.org/

Re: [Sablevm-developer] branch permissions and tag creation

[Chris P. <chr...@ma...>]

Etienne Gagnon wrote:
> Hi,
> 
> Grzegorz B. Prokopski wrote:
> 
>>Secondly, has it been agreed that this stuff is to be merged into
>>staging? I'd like to hear from Etienne before we merge.
>>(honestly, I wish I could see the diff before asking ;-)
> 
> 
> I'd like to see a comparison of this proposal with the elimination of 
> fat locks counter proposal, on performance, before we put this code (or 
> the counter proposal) in staging.

I guess we need some benchmarks before doing that (JGF?), and also a 
working fat lock elimination implementation.

For me this change is interesting mostly from a basic research 
perspective, just because it brings your algorithm a bit closer to 
Onodera's.  In my experience, it does improve performance, but not all 
of the time (sometimes it decreases performance), and the gains are not 
terribly significant.  Processor configuration has an impact, and 
presumably so does kernel version.

The other (more involved) change that would make them really comparable 
would be to implement fat lock deflation.  I expect that deflation would 
also enhance the performance benefit of this tiny "full inflation" 
change; obviously if you inflate locks more aggressively, while this 
saves you from taking the expensive inflation path some of the time, you 
do end up with more fat locks, and so you only benefit if these locks 
are under a lot of contention.

If you only want to commit this once deflation is implemented (assuming 
that happens), that's fine with me.  As it stands, it's a pretty benign 
patch, and I don't really see any problem with it (otherwise I would not 
have suggested it for staging).  I guess I'm in favour of multiple 
different algorithms being selectable at compile-time in SableVM, just 
because I think that's neat and because it shows off the suitability of 
the VM for doing research.

I do think that a paper comparing

1) Onodera's original algorithm (if SableVM can be modified to use it)
2) your original algorithm
3) your original algorithm with full lock inflation + deflation (to 
bring it closer to Onodera's)
4) this new "fat lock elimination" algorithm (Greg talked to me briefly 
about it, but I'm not clear on the details)

would be interesting.

Cheers,
Chris

Re: [Sablevm-developer] branch permissions and tag creation

[Chris P. <chr...@ma...>]

Chris Pickett wrote:
> As it stands, it's a pretty benign 
> patch, and I don't really see any problem with it (otherwise I would not 
> have suggested it for staging).

I just found that on tofu, a 2-processor SMP machine, that this change 
can cause a NullPointerException if you run a simple Incrementer.java 
benchmark enough times (attached to list some time ago).  I did not 
observe this on other machines.

However, I do not think this is a problem with my change, because all it 
does is inflate an already held lock.  Please don't interpret this as 
arrogance; I honestly don't see why the change itself would be 
responsible for this behaviour.  All I can think of is that it is 
exposing an existing problem with the locking implementation (and indeed 
I was seeing several NullPointerException problems earlier on this 
year).  Putting memory barriers in the JSR-133 cookbook-specified places 
does not fix it.  Relevant changes from the tag are attached for those 
interested without easy repository access (e.g. Clark).

I suppose we probably don't want to commit it now ... but I would 
appreciate at some point trying to nail this problem down (possibly 
meeting in person).

Cheers,
Chris

Index: src/libsablevm/thread.c
===================================================================
--- src/libsablevm/thread.c     (.../sablevm/branches/staging) 
(revision 2150)
+++ src/libsablevm/thread.c 
(.../developers/chris/tags/inflate_thin_locks)  (revision 2150)
@@ -949,6 +949,12 @@
    _svmt_JavaVM *vm = env->vm;
    _svmt_word old_lockword;

+#ifdef _SABLEVM_FULL_LOCK_INFLATION
+
+ retry:
+
+#endif /* _SABLEVM_FULL_LOCK_INFLATION */
+
    assert (instance != NULL);

    old_lockword = instance->lockword;
@@ -971,9 +977,39 @@

        if (recursive_count == 0)
         {
-         /* we're releasing the thin lock */
-         instance->lockword = _svmf_lockword_get_extra_bits (old_lockword);
-         goto handle_contention;
+
+#ifdef _SABLEVM_FULL_LOCK_INFLATION
+
+         /* CJFP
+            if the thin lock being released is under contention,
+            we now inflate it to avoid taking the slow inflation
+            path again.
+         */
+
+         if (env->contention.owner.flag)
+           {
+             if (_svmf_inflate_lock_no_exception (env, instance) != JNI_OK)
+               {
+                 _svmf_error_OutOfMemoryError (env);
+                 return JNI_ERR;
+               }
+             goto retry;
+           }
+         else
+
+#endif /* _SABLEVM_FULL_LOCK_INFLATION */
+
+           /* EMG
+              we're releasing the thin lock
+
+              CJFP
+              equivalent to line 22 in Onodera's paper
+           */
+
+           {
+             instance->lockword = _svmf_lockword_get_extra_bits 
(old_lockword);
+           }
+         goto handle_contention;
         }
        else
         {

Re: [Sablevm-developer] branch permissions and tag creation

[Chris P. <chr...@ma...>]

Grzegorz B. Prokopski wrote:
> On (30/04/04 16:22), Chris Pickett wrote:
> 
>>Hi,
>>
>>As an experiment, I am preparing a tag for Greg or David to
>>merge into staging.  It will contain a compile-time option to enable
>>inflating all contended thin locks (currently this is not done; see back
>>in the list about how in some situations it might give you a small 
>>benefit).  The option is called --enable-full-lock-inflation.
> 
> 
> Firstly, how do I get the diff of your changes? I tried: 
> 
> svn diff
> svn+ssh://svn.sablevm.org/public/developers/chris/tags/inflate_thin_locks
> -r 2146 svn+ssh://svn.sablevm.org/public/sablevm/branches/staging |less
> 
> to compare your tag to the last revision of sablevm-staging you've
> merged but it seems to be showing lots of other stuff too, like
> lt_dlopen stuff Etienne merged lately from Melanie's sandbox,
> and I can't seem to find *your* changes.

Hi Greg,

One way that works to get a diff is this:

$ svn log $CHRIS/tags
------------------------------------------------------------------------
r2150 | chris | 2004-04-30 19:07:22 -0400 (Fri, 30 Apr 2004) | 2 lines

Create tag with --enable-full-lock-inflation for merging into staging.

------------------------------------------------------------------------
r2148 | chris | 2004-04-30 18:41:45 -0400 (Fri, 30 Apr 2004) | 2 lines

Destroy bad tag.

------------------------------------------------------------------------
r2139 | chris | 2004-04-29 20:08:04 -0400 (Thu, 29 Apr 2004) | 2 lines

Create staging branch to prepare inflate_thin_locks tag.

------------------------------------------------------------------------
r820 | egagnon | 2003-10-23 20:10:05 -0400 (Thu, 23 Oct 2003) | 2 lines

Add chris developer.

------------------------------------------------------------------------

$ svn diff $STAGING@2150 $CHRIS/tags/inflate_thin_locks@2150

(i find it's very helpful to set environment variables for commonly used 
directories in your .bashrc or .profile)

Possibly there is a better way to find the last changed revision.  A 
checkout followed by svn info will also tell you.

To merge them:

$ cd staging
$ svn merge $STAGING@2150 $CHRIS/tags/inflate_thin_locks@2150
$ svn diff
[ output should be the same as previous diff, except for the headers ]

Cheers,
Chris