Bugzilla – Full Text Bug Listing
| Summary: | [enh] Add support for "application" mode in the Java 1.5+ (JVMTI) Profiler | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | z_Archived | Reporter: | Asaf Yaffe <asaf.yaffe> | ||||||||||||
| Component: | TPTP | Assignee: | Igor Alelekov <igor.alelekov> | ||||||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||||||
| Severity: | enhancement | ||||||||||||||
| Priority: | P1 | CC: | analexee, asaf.yaffe, chris.l.elford, dfayerma, jkubasta, kiryl.kazakevich, mauromol, paulslau, sluiman | ||||||||||||
| Version: | unspecified | Keywords: | plan | ||||||||||||
| Target Milestone: | --- | Flags: | asaf.yaffe:
review+
|
||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| URL: | http://www.eclipse.org/tptp/groups/Architecture/documents/features/hf_200251.html | ||||||||||||||
| Whiteboard: | closed460 | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Asaf Yaffe
Please consider for 4.5 Updated feature description URL and time estimates Approved by the AG for TPTP 4.5 with the following comments: -Sizings total 5 PW (not 4 PW). -Test sizing is too low. -Who is requiring this over the loss of function from JVMPI? -No need to have enableGC0() in the design since it is a private JNI method. -Do we need emitXMLFragments(String)? -Can the caller specify filters? Or an output mode (e.g. binary or XML)? -Is there a use case to leverage this with ProbeKit (e.g. probes calling this API). > -Test sizing is too low. I increase test time up to 1ww but reduce time for commiter work > -Who is requiring this over the loss of function from JVMPI? some post were in maillist which require this functionality > -No need to have enableGC0() in the design since it is a private JNI method. This is misprinting, disableGC() should be here. > -Do we need emitXMLFragments(String)? I think yes, service information, comments or debug messages can be added to trace over it. > -Can the caller specify filters? Or an output mode (e.g. binary or XML)? > -Is there a use case to leverage this with ProbeKit (e.g. probes calling this > API). These two points could be considered as stretch goal. Before they aren't implemented command line options should be used. Created attachment 81060 [details]
Oct. 24 Description Document
Please, commit updated version of hf_200251.html
Thanks.
Alex.
FDD committed to cvs When this is supported with PI, the idea that the data could be collected in application and standalone mode was missed. The workbench must be attached before data is collected/sent. Since everything is in theory actually controlled by the application code, it would seem that standalone mode would be a more popular scenario so that profiling data could be collected quickly and easily in cases where the workbench was not used to launch the application being monitored. Would it be a significant effort to provide both variations of application mode? Harm, Do you mean that "application mode" should be available with all JVMTI modes (Server=enabled/controlled/standalone), not exclusive as before? (In reply to comment #8) > Harm, > Do you mean that "application mode" should be available with all > JVMTI modes (Server=enabled/controlled/standalone), not exclusive as before? > Sorry for the delay. Yes. I can see and have seen equally valid use cases in all modes. *** Bug 203415 has been marked as a duplicate of this bug. *** Created attachment 87048 [details]
Feature Description
*** Bug 142986 has been marked as a duplicate of this bug. *** Created attachment 87410 [details]
patch
Asaf, would you please review the patch? Hi Igor,
Patch looks good.
Some comments:
Profiler Java class
===================
- Need Javadoc for all public methods
- getProfier: Need to provide meaningful messages for the exceptions, to help the user understand why it happened and how it can be fixed
JVMTI Runtime General Comments
==============================
- The term "application mode" appears in several places in the code. This term is anachronistic and does not reflect the implementation direction we chose (i.e., adding a new "api" flag rather than adding a new "application" operation mode). Please consider updating variable names to be consistent with the design (e.g., m_Options.isProfilerApiEnabled instead of m_Options.isApplicationMode).
Options.h/Options.cpp
=====================
- printUsage(): please change the description of the "api" argument to:
api=true|false\ <tab> Whether to enable the Profiler API (true) or not (false).
Default is false.
Option parsing: is it really necessary to rewrite the option parsing code? Please test this thoroughly to verify it does not cause any regression issues.
Profiler.h/Profiler.c
=====================
Files should be added to the build process. Need to update BuildJPIAgent32.dsp and BuildJPIAgent32.mak. To make <jni.h> (in Profiler.h) accessible to the compiler, please use the $JAVA_HOME variable in the project/make file and do not hard-code a JVM installation path. You can see examples in the Martini JPI project/make file. Also, please update the build_tptp_profiler batch/shell script to validate that $JAVA_HOME is properly defined (see example in build_tptp_martini script).
Thanks,
Asaf
Created attachment 89022 [details]
updated patch
Updated patch implementing Asaf's comments.
Are you ready to check this into Head? We are about to start the candidate driver build (In reply to comment #17) > Are you ready to check this into Head? We are about to start the candidate > driver build > Joanna, can I commit it in head now? I think we should commit this to Head now so that we can test this added function in the i5 test pass Patch applied to Head Comments to updated patch: 1. I don't see any updates to the build_tptp_profiler Windows and Linux script files (for checking the existence and validity of the JAVA_HOME variable). Since is not urgent, but should be done. 2. I don't see any updates to JPIAgent Linux makefile (src/JPIAgent/Makefile). This must be done prior to check-in, otherwise the Linux build is likely to fail. Thanks, Asaf Backed out patch Created attachment 89110 [details]
updated patch
Updated patch including required changes to the makefiles.
Tested on Linux and Windows.
Updated patch is OK. Patch applied to HEAD As of TPTP 4.6.0, TPTP is in maintenance mode and focusing on improving quality by resolving relevant enhancements/defects and increasing test coverage through test creation, automation, Build Verification Tests (BVTs), and expanded run-time execution. As part of the TPTP Bugzilla housecleaning process (see http://wiki.eclipse.org/Bugzilla_Housecleaning_Processes), this enhancement/defect is verified/closed by the Project Lead since this enhancement/defect has been resolved and unverified for more than 1 year and considered to be fixed. If this enhancement/defect is still unresolved and reproducible in the latest TPTP release (http://www.eclipse.org/tptp/home/downloads/), please re-open. As of TPTP 4.6.0, TPTP is in maintenance mode and focusing on improving quality by resolving relevant enhancements/defects and increasing test coverage through test creation, automation, Build Verification Tests (BVTs), and expanded run-time execution. As part of the TPTP Bugzilla housecleaning process (see http://wiki.eclipse.org/Bugzilla_Housecleaning_Processes), this enhancement/defect is verified/closed by the Project Lead since this enhancement/defect has been resolved and unverified for more than 1 year and considered to be fixed. If this enhancement/defect is still unresolved and reproducible in the latest TPTP release (http://www.eclipse.org/tptp/home/downloads/), please re-open. |