Tracing std::optional field?

[Xianzhu Wang]

Hi,

Compilation passed for the following code:

struct T1 {
  DISALLOW_NEW();
 public:
  void Trace(Visitor* visitor) const { visitor->Trace(member); }
  Member<LayoutObject> member;
};

struct T2 : GarbageCollected<T2> {
  void Trace(Visitor* visitor) const {}

  // { visitor->Trace(field); }  This causes compilation error.
  std::optional<T1> field;
};

TEST(X, Y) {
  HeapVector<Member<T2>> h;
}

Is `field` automatically traced (unlikely), or do we miss a check for missing trace here?

Thanks,

Xianzhu

[Jeremy Roman]

I'm pretty sure this just escapes the GC plugin; we don't automatically generate tracing code.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CADBxrifrC_6CyVqCdVt_w6sHAfDSUW8D0Cfcd%3DgVf1TDk640iw%40mail.gmail.com.

[Stefan Zager]

Does that compile? I'm surprised that you can have std::optional<T> of a type T annotated with DISALLOW_NEW().

--

[Stefan Zager]

On Fri, Feb 23, 2024 at 11:34 AM Stefan Zager <sza...@chromium.org> wrote:

Does that compile? I'm surprised that you can have std::optional<T> of a type T annotated with DISALLOW_NEW().

Never mind; it's clearly supported.

[Xianzhu Wang]

Filed crbug.com/326578830.

Will it be an optimization opportunity to support tracing through std::optional<Traceable>? Would that cost less than Member<Traceable-GarbageCollected>?

[Michael Lippautz]

Thanks for filing the plugin bugs.

Some context why this is not supported right now: Concurrent marking support is not possible with std::optional because we cannot intercept emplace() with atomics. So, we are left with

a) delegating to main thread marking;

b) introduce fine-grained locks;

b) places  a performance penalty on anything that uses the optional<Traceable>. The tradeoff then becomes allocation vs concurrent marking which is not always a clear win for the optional. c) would probably be okay if we implement something like segregated locks (We use something similar in V8, albeit only for cases much more rare than an optional would be)

-Michael

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CADBxricV-yKHuNps9x%3DONCQoeqmYF62yQzCFqC1aopBSWOsZJg%40mail.gmail.com.

[Stefan Zager]

I can't recall the exact conversation, but I think there are some strong feelings about std:optional<> vs. std::unique_ptr<> precisely due to the difference in allocation strategy. Specializing std::optional<Traceable> to do an allocation would make it redundant with unique_ptr.

How hard would it be to run an experiment to measure the difference between fine-grained locks and main thread marking?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAH%2BmL5A_Hm6Nd8W%2BfAXSWHM0XqX_sckgxU9U4aCT95zSH96cKA%40mail.gmail.com.

[David Baron]

Would it be possible to get the appropriate atomics if they were provided by a wrapper class in the middle, for example std::optional<Wrapper<Traceable>>?

-David

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAHOQ7J8A78nrPMN7gohpbpwiJSshPsRMw0SACLGJLvhJxN17Sg%40mail.gmail.com.

[Xianzhu Wang]

On Mon, Feb 26, 2024 at 11:11 AM Stefan Zager <sza...@chromium.org> wrote:

I can't recall the exact conversation, but I think there are some strong feelings about std:optional<> vs. std::unique_ptr<> precisely due to the difference in allocation strategy. Specializing std::optional<Traceable> to do an allocation would make it redundant with unique_ptr.

My thought was to use in-place allocation for std::optional<Traceable> and I wondered its feasibility. If we have to allocate, then there seems no advantage compared to Member<GarbageCollected> (thought we need to make Traceable GarbageCollected).

A data point probably related: I tested the Oilpan-PaintProperty with two variants for some heavily used temporary Vectors that serve as stacks:

1. Using (discouraged) UntracedMember in the stack entries, like

    struct StackEntry {

      UntracedMember<TransformPaintPropertyNode> transform;

      ...

    };

   Vector<StackEntry> stack;

   ...

   The UntracedMember should be safe because no paint properties can change during the lifespan of the stacks.

2. Using Member<GarbageCollected> in the stack entries, like 

   struct StackEntry : public GarbageCollected<StackEntry> {

     Member<TransformPaintPropertyNode> transform;

     ...

   };

   HeapVector<Member<StackEntry>> stack;

(Actually the CLs also have code using HeapVector<StackEntry as Traceable>, but I didn't test the variant with these vectors.)

The pinpioint job doesn't show any high-confidence difference between the two variants. It seems that Oilpan's dynamic allocation algorithm is so efficient that the cost is not even measurable in my test.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAHOQ7J8A78nrPMN7gohpbpwiJSshPsRMw0SACLGJLvhJxN17Sg%40mail.gmail.com.