This is an archive of the discontinued LLVM Phabricator instance.

Allow Java VM iterate over allocated objects
ClosedPublic

Authored by asmundak on Dec 13 2016, 10:28 AM.

Details

Reviewers
dvyukov
Summary

Objects may move during the garbage collection, and JVM needs to notify ThreadAnalyzer about that. The new function __tsan_java_find eliminates the need to maintain these objects both in ThreadAnalyzer and JVM.

Diff Detail

Event Timeline

asmundak updated this revision to Diff 81250.Dec 13 2016, 10:28 AM
asmundak retitled this revision from to Allow Java VM iterate over allocated objects .
asmundak updated this object.
asmundak added a reviewer: dvyukov.
asmundak set the repository for this revision to rL LLVM.
dvyukov edited edge metadata.Dec 14 2016, 1:43 AM

Just small nits and we are ready to land.

lib/tsan/rtl/tsan_interface_java.cc
157

Please drop "(jptr)" and { } around body.
The conversion is not necessary as jptr is some integer, and at the end of the function you do "return 0;" anyway.
Tsan codebase generally does not use { } around 1-statement if/for bodies.

... or you can drop this whole check, the condition should be handled fine by the loop (the loop will terminate instantly as "from < to" is false).

167

Wanted to ask if JVM can ever allocate 0-size blocks. But I see that we have the following checks in __tsan_java_alloc:

CHECK_NE(size, 0);
CHECK_EQ(size % kHeapAlignment, 0);

So we should be good.

lib/tsan/rtl/tsan_interface_java.h
61

s/Returns 0 there are no/Returns 0 if there are no/

test/tsan/java_find.cc
23

s/in \n/in\n/
space at the end of line

asmundak updated this revision to Diff 81403.Dec 14 2016, 9:53 AM
asmundak edited edge metadata.
asmundak removed rL LLVM as the repository for this revision.
asmundak marked 4 inline comments as done.

Addressed reviewer comments

dvyukov accepted this revision.Dec 14 2016, 10:30 AM
dvyukov edited edge metadata.

LGTM
committed in 289682

This revision is now accepted and ready to land.Dec 14 2016, 10:30 AM
dvyukov closed this revision.Dec 14 2016, 10:31 AM