RFR: 8136978 Much nearly duplicated code for vmError support (original) (raw)

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(x8664) 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 ucontextgetpc and ucontextsetpc 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::showmessagebox 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 showmessage. The two messages are named formatDebugMessage and startDebugging. The os-independet implemetation of showmessage 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