[Fix][issue-227] Native agent 10x start up - #379
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b9a3a65af
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let returned = returned.max(0).min(MAX_SCAN_FRAMES) as usize; | ||
| for frame in frames.iter().take(returned) { | ||
| if is_app_method(jvmti_env, frame.method)? { | ||
| return Ok(true); | ||
| } | ||
| } | ||
| Ok(false) |
There was a problem hiding this comment.
Fall back when the scan hits the frame cap
When an exception has more than 128 frames and the first configured app frame is deeper than this scan window, stack_has_app_frame returns false, so decide skips shouldCacheThrowable entirely even though the Java filter scans the full stack and would have cached it. In this capped case the native scan has only proven “not in the first 128 frames”, so it should return the Java fallback instead of NoAppFrame.
Useful? React with 👍 / 👎.
| let frames = build_stack_trace_frames( | ||
| jvmti_env, | ||
| jni_env, | ||
| capture, | ||
| thread, | ||
| start_depth, | ||
| num_frames.min(MAX_FRAMES), | ||
| )?; |
There was a problem hiding this comment.
Preserve stack alignment when capping cached frames
For throwables with more than 64 stack frames, this stores only the top 64 CacheFrames while BodyFactory.frames() starts matching from cachedFrames.length - 1 against the deepest Java stack element; that drains the shortened array before reaching the captured top frames, so locals are not applied at all (and shouldCacheThrowable will keep recapturing the same deep throwable because numFrames > existing.length). The cap needs to keep alignment with the Java stack or the Java merge logic needs to understand truncated arrays.
Useful? React with 👍 / 👎.
Description of the change
The JVMTI native agent made JVM startup 2–10x slower (reporter saw 15s → 400s on a Spring Boot app). Three independent causes:
Changes
Type of change
Related issues
Checklists
Development
Code review