Project

Profile

Help

Bug #4350

closed

TimingTraceListener (-TP option) has a built-in limit of 1500 on the number of stack frames

Added by Michael Kay about 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Diagnostics
Sprint/Milestone:
-
Start date:
2019-10-18
Due date:
% Done:

0%

Estimated time:
Legacy ID:
Applies to branch:
9.9, trunk
Fix Committed on Branch:
9.9, trunk
Fixed in Maintenance Release:
Platforms:

Description

The TimingTraceListener fails with an ArrayIndexOutOfBounds exception if the number of nested template/function calls exceeds 1500.

Actions #1

Updated by Michael Kay about 5 years ago

Raising the limit on stack size doesn't really help.

(a) The cost of calculating net timings rises quadratically as the stack depth increases, making the timings themselves meaningless

(b) It's not clear anyway what gross and net time mean under heavy recursion

(c) Injecting a trace listener inhibits tail recursion optimization, so we're measuring an artificial execution strategy.

I suggest that when we detect recursion, we suppress net time calculation.

Actions #2

Updated by Michael Kay about 5 years ago

It seems this stack was a rather hastily conceived solution to the problem of how to present aggregated timing data for recursive calls. It seems to be used only to detect that a call is recursive. This could have been done equally well using the existing stack (which uses java.util.Stack and therefore has no limit built in. It's a poor solution to the problem of detecting recursion because it involves a linear search of the stack to find other occurrences of the same instruction.

Replaced it with a Map<Instruction, Integer> which keeps track of the recursion depth for each active instruction.

Actions #3

Updated by Michael Kay about 5 years ago

  • Status changed from New to Resolved
  • Applies to branch 9.9, trunk added
  • Fix Committed on Branch 9.9, trunk added
Actions #4

Updated by O'Neil Delpratt almost 5 years ago

  • Status changed from Resolved to Closed
  • Fixed in Maintenance Release 9.9.1.6 added

Patch committed to the Saxon 9.9.1.6 maintenance release.

Please register to edit this issue

Also available in: Atom PDF