> On Nov 10, 2015, at 3:05 AM, Jaroslav Bachorik <jaroslav.bacho...@oracle.com> > wrote: > > Mostly nits and typos. > > hotspot/src/share/vm/classfile/javaClasses.cpp > L1941 - variable 'cpref' is not used >
Fixed. > hotspot/src/share/vm/classfile/javaClasses.inline.hpp > L117 - line number 'bci + 1000000' has a special meaning? This is existing code refactored from javaClasses.hpp. I don’t know why it added +1000000. For hidden frames, keeping line number to -1 is good to me since it means unavailable. The runtime team may have the answer. > > hotspot/src/share/vm/prims/stackwalk.cpp > L471 - typo 'pendiing' -> ‘pending' > Fixed. > src/java.base/share/classes/java/lang/LiveStackFrameInfo.java > L40,L68 - typo '""liveStackFrames"' -> '"liveStackFrames"' > L65-66 - probably a redundant info (the exact permission is listed on L67-68 > Fixed and the getStackWalker methods are moved to LiveStackFrame.java. > jdk/src/java.base/share/classes/java/lang/StackWalker.java > L222 - the documentation states only locals and operands while also monitors > are to be collected > Fixed. > jdk/src/java.base/share/classes/java/lang/StackStreamFactory.java > L478-481 - should 'initialBatchSize' be >= START_POS ? It should be >= MIN_BATCH_SIZE. I add a check there. > L574 - 'throw new IllegalStateException("origin " + origin + " != " + > skipFrames);' -> 'throw new IllegalStateException("origin " + origin + " != " > + (skipFrames + START_POS));’ > Fixed. > jdk/test/java/lang/StackWalker/VerifyStackTrace.java > L66,96,130 - 'the you can' -> 'you can’ > Fixed. > There are several '// TODO' comments in the sources and tests - is there a > plan to address them? > Yes. The tests are being cleaned up in the next revision. Mandy