Page MenuHomePhabricator

[mlir] Add a utility class, ThreadLocalCache, for storing non static thread local objects.
ClosedPublic

Authored by rriddle on Jun 25 2020, 1:36 PM.

Details

Summary

This class allows for defining thread local objects that have a set non-static lifetime. This internals of the cache use a static thread_local map between the various different non-static objects and the desired value type. When a non-static object destructs, it simply nulls out the entry in the static map. This will leave an entry in the map, but erase any of the data for the associated value. The current use cases for this are in the MLIRContext, meaning that the number of items in the static map is ~1-2 which aren't particularly costly enough to warrant the complexity of pruning. If a use case arises that requires pruning of the map, the functionality can be added.

This is especially useful in the context of MLIR for implementing thread-local caching of context level objects that would otherwise have very high lock contention. This revision adds a thread local cache in the MLIRContext for attributes, identifiers, and types to reduce some of the locking burden. This led to a speedup of several hundred miliseconds when compiling a conversion pass on a very large mlir module(>300K operations).

Depends On D82596

Diff Detail

Event Timeline

rriddle created this revision.Jun 25 2020, 1:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2020, 1:36 PM
rriddle updated this revision to Diff 283451.Aug 5 2020, 5:18 PM

Switch to a combo of weak_ptr/shared_ptr.

mehdi_amini accepted this revision.Aug 5 2020, 9:17 PM

Nice!

mlir/include/mlir/Support/ThreadLocalCache.h
56
if (std::shared_ptr<ValueT> value = threadInstance.lock())
  return *value;

?

(It makes it more explicit that there isn't a TOCTOU issue here, it is also the common pattern I think)

mlir/lib/IR/MLIRContext.cpp
657–660

Can you split the assignment from the call?
I am not convinced the side-effect in constructing the returned value is helping the readability.

This revision is now accepted and ready to land.Aug 5 2020, 9:17 PM
rriddle updated this revision to Diff 283737.Aug 6 2020, 2:44 PM
rriddle marked an inline comment as done.

Resolve comments

Harbormaster completed remote builds in B67525: Diff 284022.
This revision was landed with ongoing or failed builds.Aug 7 2020, 1:44 PM
This revision was automatically updated to reflect the committed changes.