Hi David, On 12.11.2014 07:48, David Holmes wrote: > On 12/11/2014 12:40 AM, Aleksey Shipilev wrote: > All looks good to me.
Thanks for the review! > But I also noticed this strange (to me) assertion in javaClasses.cpp > > void java_lang_Thread::set_name(oop java_thread, oop name) { > assert(java_thread->obj_field(_name_offset) == NULL, "name should be > NULL"); > java_thread->obj_field_put(_name_offset, name); > } > > and on investigation it seems like this is dead code - I couldn't locate > a call to java_lang_Thread::set_name ?? It would only be usable on an > attaching thread (else name can't be null) and we pass the name to the > Thread constructor in that case. set_name is not used, as I mentioned earlier -- that makes the change even more "safe". I was even tempted to drop the setter completely, but it would break the symmetry against other setters and getters. I dropped the assert at set_name in this update: http://cr.openjdk.java.net/~shade/8059677/webrev.03.hs/ http://cr.openjdk.java.net/~shade/8059677/webrev.03.jdk/ The only difference against the previous version is the dropped assert, so I haven't re-spinned the tests. Thanks, -Aleksey.