RFR (L): Add TSAN instrumentation for allocation and garbage collection
Arthur Eubanks
aeubanks at google.com
Fri Jun 28 00:45:50 UTC 2019
Could you change the comment in tsanOopMap.hpp
39 // 1. add() is only passed a live oop.
40 // 2. add() must be thread-safe wrt itself.
"add()" -> "add_*()"
Another comment in tsanExternalDecls.hpp which seems inaccurate
+ // Called lazily when an oop should start to be tracked by Tsan.
It seems like we're calling __tsan_java_alloc() eagerly, not lazily.
tsanOopMap.cpp:
We fixed this, comment is wrong:
448 // TODO: We never call __tsan_java_fini; we should.
And right above, maybe we could deallocate where we call __tsan_java_fini?
447 // TODO: We never deallocate the oopMap; we should.
Should this instead be an assert and we do the check for ThreadSanitizer in
the caller?
456 if (!ThreadSanitizer) return;
I also need more time to review rebuild_oops_map() :).
On Thu, Jun 27, 2019 at 2:49 PM Jean Christophe Beyler <jcbeyler at google.com>
wrote:
> Hi Man,
>
>
> https://cr.openjdk.java.net/~manc/tsanOopMap/webrev.00/src/hotspot/share/runtime/sharedRuntime.cpp.udiff.html
> // Note Well.
> ->
> // NOTE:
>
>
> https://cr.openjdk.java.net/~manc/tsanOopMap/webrev.00/test/hotspot/jtreg/tsan/NonRacyGarbageCollectionLoopTest.java.html
> -> I would put a note somewhere that if you change the 40 for HEAP_SIZE you
> should change the Xms/Xmx... (I imagined we could create the Xms/Xmx with
> string concatenation but that seems overkill)
>
>
> https://cr.openjdk.java.net/~manc/tsanOopMap/webrev.00/src/hotspot/share/tsan/tsanOopMap.cpp.html
> - I find TsanOopBucket is not the right name, it might be TsanOop
>
> - 120 do {
> 121 h = hash(h);
> 122 bucket = &buckets[h % _size];
> 123 } while (bucket->has_oop() && bucket->get_oop() != o);
>
> This could be put in a helper method because done twice, no?
>
> - print_map is not used, is it?
> - oopMap should be oop_map no?
>
> I admit I have not finished looking at rebuild_oops_map, it is a bit big
> ;-) could we break it up?
>
> Thanks,
> Jc
>
More information about the tsan-dev
mailing list