RFR: 8136978 Much nearly duplicated code for vmError support

David Holmes david.holmes at oracle.com
Thu Nov 19 02:40:46 UTC 2015


Hi Sebastien,

On 19/11/2015 12:00 AM, Sebastian Sickelmann wrote:
> Hi, I rebased the patch to tip of  hs-rt and prepared a changeset for
> the case the review is positive.
>
> I also worked in most of the suggestions from Thomas.
>
> The new webrev/changeset is here:
> http://cr.openjdk.java.net/~sebastian/8136978/webrev.01/
>
> The used
>
> 	Reviewed-by: stuefe, coleenp
>
> is pending on an positive review.
>
> What is the normal way:
>
> Does a author without commit(push) rights create changesets?

Yes that is the definition of the Author role. Unfortunately we do have 
a terminology conflict between the "hg commit" (which an Author can do) 
and the "hg push" (which requires a Committer).

> I see some problems with outstanding results from reviews.

Once the reviews are finalized you create the (final) changeset with all 
the appropriate attributions.

You don't need to create a changeset to generate a webrev.

Thanks,
David
-----


> Thanks for your help.
> -- Sebastian
>
>
>
> On 11/18/2015 09:14 AM, Thomas Stüfe wrote:
>> Hi Sebastian,
>>
>> I will take a look at the changes and test on AIX. Give me a day or two.
>>
>> Kind Regards, Thomas
>>
>> On Tue, Nov 17, 2015 at 9:53 PM, Coleen Phillimore
>> <coleen.phillimore at oracle.com <mailto:coleen.phillimore at oracle.com>>
>> wrote:
>>
>>
>>      Hi Sebastian,
>>
>>      I think this change looks very good.  I can test the patch on all
>>      the platforms that we have, and let you know if it needs
>>      adjustment.  Maybe someone with aix can test there too.  Thank you
>>      for the change removing this duplicated code.
>>
>>      Thanks,
>>      Coleen
>>
>>
>>      On 10/16/15 12:33 AM, Sebastian Sickelmann wrote:
>>
>>          Hi,
>>
>>          i have looked at the enhancement JDK-8136978, please find my
>>          first suggestion
>>          at [0]http://cr.openjdk.java.net/~sebastian/8136978/webrev.00/
>>          <http://cr.openjdk.java.net/%7Esebastian/8136978/webrev.00/>.
>>
>>          I first looked also at another solution but don't liked it.
>>          For some details
>>          on this and comments of Coleen and Kim to this, see the thread
>>          at hotspot-dev[1].
>>
>>          Right now i only compile-"tested" it on my linux(x86_64)
>>          machine, so there might
>>          be some error in aix,bsd,solaris. I am not sure if i can setup
>>          my machine to compile
>>          those as well. For the windows-part i am actually also not
>>          able to compile or test it.
>>          I think there is a good change to optimize the use of
>>          #include's in the changed files,
>>          but I am not sure how i can effectively work out where i can
>>          reduce some imports.
>>
>>          There are no additional tests for this. What is the best way
>>          to really test such a change
>>          on all platforms. Do you use your development-machine for
>>          this, or is there some
>>          infrastructure that can test such multi-platform changes for you?
>>
>>          Here is a short description of the suggested change:
>>
>>          Nearly identically implementations of VMError moved from the
>>          os/[linux|aix|bsd|solaris]
>>          to a os/posix. The parts that are different were refactored
>>          and are now in the os-specific
>>          implementations of the os class. The two os specific methods
>>          ucontext_get_pc and
>>          ucontext_set_pc are moved to the declaration of the os::Posix
>>          class. The implementations
>>          of those remain in the os_[linux|aix|bsd|solaris].cpp
>>          implementations but are renamed
>>          acordingly. All uses of these methods are replaces to use the
>>          "os::Poxis prefix".
>>
>>          For the method VMError::show_message_box also the windows
>>          implementation is changed. Now
>>          there are two methods in the declarartion of the class os that
>>          are used to help the
>>          os-independent implementation of show_message. The two
>>          messages are named formatDebugMessage
>>          and startDebugging. The os-independet implemetation of
>>          show_message can be found in
>>          share/vm/utilities/vmError.cpp
>>          -- Sebastian
>>
>>
>>          [0]http://cr.openjdk.java.net/~sebastian/8136978/webrev.00/
>>          <http://cr.openjdk.java.net/%7Esebastian/8136978/webrev.00/>
>>          [1]http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-October/020249.html
>>          suggestion to this.
>>
>>
>>
>>
>


More information about the hotspot-runtime-dev mailing list