RFR: 8227745: Enable Escape Analysis for Better Performance in the Presence of JVMTI Agents
Serguei Spitsyn
sspitsyn at openjdk.java.net
Tue Sep 22 01:37:23 UTC 2020
On Tue, 22 Sep 2020 00:38:43 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:
>> Reviewed in the good old hg times :)
>> See also http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-August/041453.html
>
> Hi Richard,
>
> Could you consider to place the classes EscapeBarrier and JvmtiDeferredUpdates into theyr own .hpp/.cpp files? The
> class JvmtiDeferredUpdates would be better to put into the folder 'prims' then.
> src/hotspot/share/opto/macro.cpp:
> @@ -1091,11 +1091,11 @@
> bool PhaseMacroExpand::eliminate_allocate_node(AllocateNode *alloc) {
> // Don't do scalar replacement if the frame can be popped by JVMTI:
> // if reallocation fails during deoptimization we'll pop all
> // interpreter frames for this compiled frame and that won't play
> // nice with JVMTI popframe.
> - if (!EliminateAllocations || JvmtiExport::can_pop_frame() || !alloc->_is_non_escaping) {
> + if (!EliminateAllocations || !alloc->_is_non_escaping) {
> return false;
> }
>
> I wonder if the comment is still correct after you removed the check for JvmtiExport::can_pop_frame().
>
> src/hotspot/share/runtime/deoptimization.hpp:
>
> + EscapeBarrier(JavaThread* calling_thread, JavaThread* deoptee_thread, bool barrier_active)
> + : _calling_thread(calling_thread), _deoptee_thread(deoptee_thread),
> + _barrier_active(barrier_active && (JVMCI_ONLY(UseJVMCICompiler) NOT_JVMCI(false)
> + COMPILER2_PRESENT(|| DoEscapeAnalysis)))
> . . . . . . . . .
> +
> + // Revert ea based optimizations for all java threads
> + EscapeBarrier(JavaThread* calling_thread, bool barrier_active)
> + : _calling_thread(calling_thread), _deoptee_thread(NULL),
>
> Nit: would better to make the parameter deoptee_thread to be the 3rd to better mach the seconf constructor.
>
> + bool all_threads() const { return _deoptee_thread == NULL; } // Should revert optimizations for all
> threads. + bool self_deopt() const { return _calling_thread == _deoptee_thread; } // Current thread deoptimizes
> its own objects. + bool barrier_active() const { return _barrier_active; } // Inactive barriers are
> created if no local objects can escape.
> I'd suggest to put comments in a line before function definitions as it is done for other declarations/definitions.
src/hotspot/share/runtime/deoptimization.cpp:
@@ -349,12 +408,12 @@
// Now that the vframeArray has been created if we have any deferred local writes
// added by jvmti then we can free up that structure as the data is now in the
// vframeArray
- if (thread->deferred_locals() != NULL) {
- GrowableArray<jvmtiDeferredLocalVariableSet*>* list = thread->deferred_locals();
+ if (JvmtiDeferredUpdates::deferred_locals(thread) != NULL) {
+ GrowableArray<jvmtiDeferredLocalVariableSet*>* list = JvmtiDeferredUpdates::deferred_locals(thread);
int i = 0;
do {
// Because of inlining we could have multiple vframes for a single frame
// and several of the vframes could have deferred writes. Find them all.
if (list->at(i)->id() == array->original().id()) {
@@ -365,13 +424,14 @@
} else {
i++;
}
} while ( i < list->length() );
if (list->length() == 0) {
- thread->set_deferred_locals(NULL);
- // free the list and elements back to C heap.
- delete list;
+ JvmtiDeferredUpdates* updates = thread->deferred_updates();
+ thread->set_deferred_updates(NULL);
+ // free deferred updates.
+ delete updates;
}
It is not clear why the 'list' is not deleted anymore. If it is intentional then could you, please, add a comment with
an explanation?
-------------
PR: https://git.openjdk.java.net/jdk/pull/119
More information about the serviceability-dev
mailing list