Re: std::function

[Daniel Cheng]

The guidelines for C++ usage in Blink are the same as the guidelines for C++ usage in Chrome, so std::function is banned in Blink =)

1) It would be useful to understand where the performance difference between base::Callback and std::function are coming from: my guess is base::Callback involves more indirections, since the BindState is always heap-allocated. In contrast, std::function doesn't always require going to the heap.

2) A discussion of whether or not to allow std::function probably belongs on c...@chromium.org (which I've added to this thread).

Daniel

On Tue, Sep 11, 2018 at 11:50 AM Emil A Eklund <e...@chromium.org> wrote:

Hi,

What's our policy on using std::function in Blink? We have a number of
uses of it in blink code already but the Chromium guide explicitly
forbids it [1].

The reason I'm asking is because the recommended alternative,
base::Callback, is *significantly* slower. We're seeing a 5-10%
regression on some of the layout performance tests when using
base::Callback but a slight improvement when using std::function to
replace the same lambda [2] on both high- and low-end devices where
ArabicLineLayout gets ~12% slower with base::Callback [3] and ~1%
faster with std::function [4] .

If the ban applies, what's the recommended way to implement a
synchronous typed callback function? The base::Callback overhead is
somewhat problematic in this case.

Thanks

1: https://chromium-cpp.appspot.com/#library-blacklist
2: https://chromium-review.googlesource.com/c/chromium/src/+/1208728/4..5
3: https://pinpoint-dot-chromeperf.appspot.com/results2/1395c415640000
4: https://pinpoint-dot-chromeperf.appspot.com/results2/109cf38d640000

--
Emil

--
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 post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CADu_oUD%3DTqDB%3D0pjK9kwQaj3yhKyrTD7WWCFegO03LfgYWNZrg%40mail.gmail.com.

[Daniel Cheng]

On Tue, Sep 11, 2018 at 12:00 PM Daniel Cheng <dch...@chromium.org> wrote:

The guidelines for C++ usage in Blink are the same as the guidelines for C++ usage in Chrome, so std::function is banned in Blink =)

1) It would be useful to understand where the performance difference between base::Callback and std::function are coming from: my guess is base::Callback involves more indirections, since the BindState is always heap-allocated. In contrast, std::function doesn't always require going to the heap.

2) A discussion of whether or not to allow std::function probably belongs on c...@chromium.org (which I've added to this thread).

Daniel

On Tue, Sep 11, 2018 at 11:50 AM Emil A Eklund <e...@chromium.org> wrote:

Hi,

What's our policy on using std::function in Blink? We have a number of
uses of it in blink code already but the Chromium guide explicitly
forbids it [1].

The reason I'm asking is because the recommended alternative,
base::Callback, is *significantly* slower. We're seeing a 5-10%
regression on some of the layout performance tests when using
base::Callback but a slight improvement when using std::function to
replace the same lambda [2] on both high- and low-end devices where
ArabicLineLayout gets ~12% slower with base::Callback [3] and ~1%
faster with std::function [4] .

If the ban applies, what's the recommended way to implement a
synchronous typed callback function? The base::Callback overhead is
somewhat problematic in this case.

Sorry I realize I didn't answer this question. I think the most common patterns (that don't involve std::function) are:

- pass a function pointer

- accept the functor as a template parameter so lambdas can be passed

I'm guessing the latter doesn't work well for layout, but could the former be an option?

Daniel

[dan...@chromium.org]

On Tue, Sep 11, 2018 at 3:07 PM Daniel Cheng <dch...@chromium.org> wrote:

On Tue, Sep 11, 2018 at 12:00 PM Daniel Cheng <dch...@chromium.org> wrote:

The guidelines for C++ usage in Blink are the same as the guidelines for C++ usage in Chrome, so std::function is banned in Blink =)

1) It would be useful to understand where the performance difference between base::Callback and std::function are coming from: my guess is base::Callback involves more indirections, since the BindState is always heap-allocated. In contrast, std::function doesn't always require going to the heap.

2) A discussion of whether or not to allow std::function probably belongs on c...@chromium.org (which I've added to this thread).

Daniel

On Tue, Sep 11, 2018 at 11:50 AM Emil A Eklund <e...@chromium.org> wrote:

Hi,

What's our policy on using std::function in Blink? We have a number of
uses of it in blink code already but the Chromium guide explicitly
forbids it [1].

The reason I'm asking is because the recommended alternative,
base::Callback, is *significantly* slower. We're seeing a 5-10%
regression on some of the layout performance tests when using
base::Callback but a slight improvement when using std::function to
replace the same lambda [2] on both high- and low-end devices where
ArabicLineLayout gets ~12% slower with base::Callback [3] and ~1%
faster with std::function [4] .

If the ban applies, what's the recommended way to implement a
synchronous typed callback function? The base::Callback overhead is
somewhat problematic in this case.

Sorry I realize I didn't answer this question. I think the most common patterns (that don't involve std::function) are:

- pass a function pointer

- accept the functor as a template parameter so lambdas can be passed

Some more alternatives because I generally don't like Callbacks that are only used synchronously...

Callbacks (and function) are useful for erasing types and binding additional state into the callback. This scenario seems like perhaps a virtual interface would be more efficient and more clear to read in the code (less indirection to an unknown type at the callsite to the Callback/std::function). It looks like it would just mean passing the const StringView& through to the ForEachGlyph() function for it to pass along to the interface's AddGlyph().

Another approach would be to return an iterator from ForEachGlyph instead of calling a function repeatedly inside it, which avoids re-entering ShapeResultBloberizer at all, which may be a win for you also.

 

I'm guessing the latter doesn't work well for layout, but could the former be an option?

Daniel

 

Thanks

1: https://chromium-cpp.appspot.com/#library-blacklist
2: https://chromium-review.googlesource.com/c/chromium/src/+/1208728/4..5
3: https://pinpoint-dot-chromeperf.appspot.com/results2/1395c415640000
4: https://pinpoint-dot-chromeperf.appspot.com/results2/109cf38d640000

--
Emil

--
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 post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CADu_oUD%3DTqDB%3D0pjK9kwQaj3yhKyrTD7WWCFegO03LfgYWNZrg%40mail.gmail.com.

--
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 post to this group, send email to platform-arc...@chromium.org.

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

[Emil A Eklund]

On Tue, Sep 11, 2018 at 12:07 PM, Daniel Cheng <dch...@chromium.org> wrote:
> On Tue, Sep 11, 2018 at 12:00 PM Daniel Cheng <dch...@chromium.org> wrote:
>>
>> The guidelines for C++ usage in Blink are the same as the guidelines for
>> C++ usage in Chrome, so std::function is banned in Blink =)

That's quite disappointing but thanks for the confirmation.

> Sorry I realize I didn't answer this question. I think the most common
> patterns (that don't involve std::function) are:
> - pass a function pointer

Yeah that's what I'm leaning towards.

> - accept the functor as a template parameter so lambdas can be passed

Lambdas don't work across compilation units and make for a very
awkward API (the point of the linked change is to replace a lambda).

> Another approach would be to return an iterator from ForEachGlyph instead
> of calling a function repeatedly inside it, which avoids re-entering
> ShapeResultBloberizer at all, which may be a win for you also.

Was the first thing I tried and it requires a lot more code and is
only slightly faster than a callback based approach.

I was hoping that there would be some magic bit I could set on
base:Callback to avoid the overhead but it's seems I'm out of luck

Thanks for the background and suggestions everyone!

--
Emil

[Albert J. Wong (王重傑)]

I am actually a bit curious about this though.  Do you know if it is base::Callback's Run() being slow,

or is it the base::Bind() being slow?

If it's ::Run(), I'd like to know....

 

Thanks for the background and suggestions everyone!

--
Emil

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CADu_oUCx6bp0yodEx4Un5dLxSRWfFZtOGJkZVsmPBcK1JDPSfQ%40mail.gmail.com.

[Emil A Eklund]

No idea, how would one use one without the other?

[dan...@chromium.org]

Since you have a test that reproduces this, you could insert the Bind but not use it, to determine what % is the Bind vs the Bind+Run. It wouldn't be something useful, of course :)

 

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CADu_oUBL4V5VE2Y-Y3YNPwLaazXzcJJ0HiK%2BmxoddZWQpZ4x-w%40mail.gmail.com.

[Albert J. Wong (王重傑)]

You can't generally but for a perf test you could hack it so the BindRepeating() call is only done once outside of the performance critical path. Then rather than binding in "this" and "text" to the context, just pass it through as a function argument to ForEachGlyph.

IIRC (and it's a fuzzy memory) the Callback::Run() should compile out to nearly a raw function-pointer call. std::function() should be similar too... so I would expect largest place for a divergence to occur would be in the allocation and atomicity guards that happen during binding.

[Vladimir Levin]

There seems to be about a dozen non-test uses of std::function in Chromium and similar amount in Blink, should those be fixed?

Alternatively, since there seems to be a performance difference between std::function and base::Callback, should we consider "unbanning" but discouraging std::function? As far as I know, one of the driving decisions for banning std::function was that base::Callback is a better replacement, but it seems that on the performance front that's not the case (this is a bit on a tangent from figuring out _why_ there is a performance difference)

--
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 post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CALcbsXANqs3UDdzj%3DJ-N8AaUTCx%2BBKxi2bX99X018t2%3DJPeUyA%40mail.gmail.com.

[Albert J. Wong (王重傑)]

On Tue, Sep 11, 2018 at 2:44 PM Vladimir Levin <vmp...@chromium.org> wrote:

There seems to be about a dozen non-test uses of std::function in Chromium and similar amount in Blink, should those be fixed?

Unless the policy changes, yes.

 

Alternatively, since there seems to be a performance difference between std::function and base::Callback, should we consider "unbanning" but discouraging std::function? As far as I know, one of the driving decisions for banning std::function was that base::Callback is a better replacement, but it seems that on the performance front that's not the case (this is a bit on a tangent from figuring out _why_ there is a performance difference)

^^^ That's actually why I'm asking the question about performance. If the performance divergence is executing an std::function is faster than executing a base::Callback<>, and we cannot make base::Callback<> match, then there might be a reason to explore unbanning std::function.

However, if the divergence is in std::bind() vs base::Bind(), then that's less compelling as std::bind() is significantly different enough that I doubt we'd want to allow that.

Without knowing where the slow-down is coming in from (Callback itself, or the Binding) it's hard to discussion unbanning std::function<> sensibly.

-Albert

[Emil A Eklund]

I'll run some experiments and see what I can come up with.

[Vladimir Levin]

On Tue, Sep 11, 2018, 14:50 Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Tue, Sep 11, 2018 at 2:44 PM Vladimir Levin <vmp...@chromium.org> wrote:

There seems to be about a dozen non-test uses of std::function in Chromium and similar amount in Blink, should those be fixed?

Unless the policy changes, yes.

 

Alternatively, since there seems to be a performance difference between std::function and base::Callback, should we consider "unbanning" but discouraging std::function? As far as I know, one of the driving decisions for banning std::function was that base::Callback is a better replacement, but it seems that on the performance front that's not the case (this is a bit on a tangent from figuring out _why_ there is a performance difference)

^^^ That's actually why I'm asking the question about performance. If the performance divergence is executing an std::function is faster than executing a base::Callback<>, and we cannot make base::Callback<> match, then there might be a reason to explore unbanning std::function.

However, if the divergence is in std::bind() vs base::Bind(), then that's less compelling as std::bind() is significantly different enough that I doubt we'd want to allow that.

 

From my naive local tests, it seems that invoking a base::RepeatingCallback and invoking a std::function have similar performance. However, base::Bind is about a magnitude slower than creating std::function with std::bind or with a stateful lambda.

Is it possible to consider allowing std::function without allowing std::bind? The difference as I see it is that the only way (that I know of) to create a non-empty base::Callback is via base::Bind. Whereas std::function can be constructed out of a stateful lambda.

I'm mostly asking if std::function could be allowed for simple callback case where we would right now have to either templatize a function in order to accept a stateful lambda, or use both base::Bind and base::Callback. Of course the same rules about ensuring that std::function doesn't outlive the stack of the caller would apply.

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALcbsXCzoZRhLQ-mx0ogMu870ieRKNYUynfYzTJawbQNLhGR0g%40mail.gmail.com.

[Albert J. Wong (王重傑)]

On Tue, Sep 11, 2018 at 4:22 PM Vladimir Levin <vmp...@chromium.org> wrote:

On Tue, Sep 11, 2018, 14:50 Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Tue, Sep 11, 2018 at 2:44 PM Vladimir Levin <vmp...@chromium.org> wrote:

There seems to be about a dozen non-test uses of std::function in Chromium and similar amount in Blink, should those be fixed?

Unless the policy changes, yes.

 

Alternatively, since there seems to be a performance difference between std::function and base::Callback, should we consider "unbanning" but discouraging std::function? As far as I know, one of the driving decisions for banning std::function was that base::Callback is a better replacement, but it seems that on the performance front that's not the case (this is a bit on a tangent from figuring out _why_ there is a performance difference)

^^^ That's actually why I'm asking the question about performance. If the performance divergence is executing an std::function is faster than executing a base::Callback<>, and we cannot make base::Callback<> match, then there might be a reason to explore unbanning std::function.

However, if the divergence is in std::bind() vs base::Bind(), then that's less compelling as std::bind() is significantly different enough that I doubt we'd want to allow that.

 

From my naive local tests, it seems that invoking a base::RepeatingCallback and invoking a std::function have similar performance. However, base::Bind is about a magnitude slower than creating std::function with std::bind or with a stateful lambda.

That matches my expectations. IIRC, the big difference is std::bind() doesn't allocate.

 

Is it possible to consider allowing std::function without allowing std::bind? The difference as I see it is that the only way (that I know of) to create a non-empty base::Callback is via base::Bind. Whereas std::function can be constructed out of a stateful lambda.

It's been years since I looked at this, but my first thought is to create a BindStatic() that somehow produced base::Callback<> variant for simple cases (eg, no bound variables). The problem to solve doesn't seem to be allowing std::function as much as it is allow something that can hold a lambda or raw function w/o as much overhead.

<not-well-thought-out-digression>

I wonder for example if you could create hacked-up region-based memory management concept within which you could attach a lambda that puts itself into a crash-on-call state.  Imagine an API as follows.

{

  Region r;  //

  base::RepeatingCallback<void(void)> cb = r.MakeCallback( /* some capturing lambda here */)

  synchronous_api(cb);

}

Region::MakeCallback() would return something that can be stored in a base::Callback<>, but at the end of the region would NULL out the bind_state_ or something like that.

</not-well-thought-out-digression>

-Albert

[Albert J. Wong (王重傑)]

On Tue, Sep 11, 2018 at 4:30 PM Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Tue, Sep 11, 2018 at 4:22 PM Vladimir Levin <vmp...@chromium.org> wrote:

On Tue, Sep 11, 2018, 14:50 Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Tue, Sep 11, 2018 at 2:44 PM Vladimir Levin <vmp...@chromium.org> wrote:

There seems to be about a dozen non-test uses of std::function in Chromium and similar amount in Blink, should those be fixed?

Unless the policy changes, yes.

 

Alternatively, since there seems to be a performance difference between std::function and base::Callback, should we consider "unbanning" but discouraging std::function? As far as I know, one of the driving decisions for banning std::function was that base::Callback is a better replacement, but it seems that on the performance front that's not the case (this is a bit on a tangent from figuring out _why_ there is a performance difference)

^^^ That's actually why I'm asking the question about performance. If the performance divergence is executing an std::function is faster than executing a base::Callback<>, and we cannot make base::Callback<> match, then there might be a reason to explore unbanning std::function.

However, if the divergence is in std::bind() vs base::Bind(), then that's less compelling as std::bind() is significantly different enough that I doubt we'd want to allow that.

 

From my naive local tests, it seems that invoking a base::RepeatingCallback and invoking a std::function have similar performance. However, base::Bind is about a magnitude slower than creating std::function with std::bind or with a stateful lambda.

That matches my expectations. IIRC, the big difference is std::bind() doesn't allocate.

 

Is it possible to consider allowing std::function without allowing std::bind? The difference as I see it is that the only way (that I know of) to create a non-empty base::Callback is via base::Bind. Whereas std::function can be constructed out of a stateful lambda.

It's been years since I looked at this, but my first thought is to create a BindStatic() that somehow produced base::Callback<> variant for simple cases (eg, no bound variables). The problem to solve doesn't seem to be allowing std::function as much as it is allow something that can hold a lambda or raw function w/o as much overhead.

<not-well-thought-out-digression>

I wonder for example if you could create hacked-up region-based memory management concept within which you could attach a lambda that puts itself into a crash-on-call state.  Imagine an API as follows.

{

  Region r;  //

  base::RepeatingCallback<void(void)> cb = r.MakeCallback( /* some capturing lambda here */)

  synchronous_api(cb);

}

Region::MakeCallback() would return something that can be stored in a base::Callback<>, but at the end of the region would NULL out the bind_state_ or something like that.

</not-well-thought-out-digression>

Probably not possible to make it fully memory safe, but you might be able to handle some sort of dcheck to ensure the refcount is zero...and even if you couldn't provide "real" safety, it'd mark the area of code as requiring scrutiny.

[Daniel Cheng]

std::function has a number of gotchas that make me hesitant to allow both (at least without better guidelines/recommendations of when std::function is appropriate).

- std::function can only be copied, not moved. If the functor it captures has a significant amount of state, this copy is expensive.

- std::function requires that its functor be copyable, so move-only types simply won't work with std::function.

- std::function can be slower than just using lambdas (or even function calls), probably depending on whether or not you have to heap alloc.

Daniel

On Tue, Sep 11, 2018 at 2:44 PM Vladimir Levin <vmp...@chromium.org> wrote:

[Emil A Eklund]

On Wed, Sep 12, 2018 at 3:07 AM, Daniel Cheng <dch...@chromium.org> wrote:
> std::function has a number of gotchas that make me hesitant to allow both
> (at least without better guidelines/recommendations of when std::function is
> appropriate).
>
> - std::function can only be copied, not moved. If the functor it captures
> has a significant amount of state, this copy is expensive.
> - std::function requires that its functor be copyable, so move-only types
> simply won't work with std::function.
> - std::function can be slower than just using lambdas (or even function
> calls), probably depending on whether or not you have to heap alloc.

Following up. The speed regression I observed between std::function
and base::Callback was, in fact, due to base::Bind being significantly
slower than the std version. I was not able to detect any
statistically significant difference in performance between
base::Callback and std::function.

In reality this destination might not really matter though as all
non-trivial uses of Callback requires base::Bind.

Allowing std::function without allowing std::bind wouldn't really get
us anything as far as I can tell. Allowing std::bind on the other hand
would allow for some nice speedups but I understand that is unlikely
to happen...

Thanks for all your explanations and the time you've all spent
answering my questions. I really appreciate it.

[Daniel Cheng]

One can still use capturing lambdas with std::function though. Granted, that was something we explicitly decided to disallow with base::Callback, but if we had some way of guarantee that capturing lambdas weren't run outside their original creation scope...

Daniel

[Albert J. Wong (王重傑)]

...working on it...

Basically, you should be able to do something like

int foo = 5;

{

  auto callback_region = AdaptLambda([&foo](int n) { return foo + n; });

  base::Callback<int(int)> cb = callback_region.ToCallback();
  DoFunc(cb);

} // CHECK(cb.HasOneRef) on destruction of callback_region.

[Albert J. Wong (王重傑)]

Specifically, something like this:

  https://chromium-review.googlesource.com/c/chromium/src/+/1220629

...and now that it's 10:45... off to do laundry, paint the deck, and then bed.

[Yuri Wiitala]

OOC, does base::Bind *need* to allocate the bind state on the heap? Would it cause problems for it to be a data member in the Callback class itself?

2018年9月11日(火) 22:46 Albert J. Wong (王重傑) <ajw...@chromium.org>:

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALcbsXDJCVQ1wpsUJYtz_hkyS%2ByXMMPiruXaZmEH53Am5FvAWA%40mail.gmail.com.

[Albert J. Wong (王重傑)]

On Wed, Sep 12, 2018 at 11:53 AM Yuri Wiitala <m...@chromium.org> wrote:

OOC, does base::Bind *need* to allocate the bind state on the heap? Would it cause problems for it to be a data member in the Callback class itself?

That's essentially what my hack CL did. In general, I think it's impossible to do this and maintain a nice API. Callback/Bind has 2 major concepts:

Callback - Generic, type-erased holder. It knows its runtime and nothing else. Corollary: the size is constant

Bind - The main engine for type inference. It analyzes the arguments given to create a correctly sized BindState. This is why it dynamically allocates.

If we want to put the storage on the stack, then we need to be able to, statically, name a type with sufficient storage to hold

the bound parameters which doesn't seem easy.  Hypothetically if we created a StackCallback<size_t, RunType> and wanted to be able to write stuff

like:

  void Foo(int, int);

  StackCallback<right_number_here, void(void)> cb = base::StackBind(&Foo, 1, 2);

Then you end up in a paradox where, even though |right_number_here| can be determined by the compiler, you cannot write it in code

because it's based on examining the expression resulting from base::StackBind(&Foo, 1, 2);

The closest I can come up with is using auto.

The only other way I can think of doing it is allocating a fixed sized buffer in StackCallback<> and static_assert-ing that the size of the

resulting BindState is never larger.

But in all cases, it doesn't replace the general base::Bind()/base::Callback<> usage as the lifetime semantics are too different.

-Albert

[Łukasz Anforowicz]

On Wed, Sep 12, 2018 at 1:08 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Wed, Sep 12, 2018 at 11:53 AM Yuri Wiitala <m...@chromium.org> wrote:

OOC, does base::Bind *need* to allocate the bind state on the heap? Would it cause problems for it to be a data member in the Callback class itself?

That's essentially what my hack CL did. In general, I think it's impossible to do this and maintain a nice API. Callback/Bind has 2 major concepts:

Callback - Generic, type-erased holder. It knows its runtime and nothing else. Corollary: the size is constant

Bind - The main engine for type inference. It analyzes the arguments given to create a correctly sized BindState. This is why it dynamically allocates.

If we want to put the storage on the stack, then we need to be able to, statically, name a type with sufficient storage to hold

the bound parameters which doesn't seem easy.

Crazy idea: what if BindStateStorage was like std::string which does small string optimization:

class BindStateStorage {

 public:

  BindState* get();

 private:

  union {

    char[kSmallBindStateSize] small_state_;

    BindState* big_state_;

  }

  bool is_small_;

};

I have no idea how one would calculate kSmallBindStateSize.  Overall memory usage would increase...

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALcbsXAiJ8QWOHL4KE_5Z_v1Q0Xi82w-%2BH2UXV3jSqQt8ETZ%3DQ%40mail.gmail.com.

[Albert J. Wong (王重傑)]

On Wed, Sep 12, 2018 at 2:39 PM Łukasz Anforowicz <luk...@chromium.org> wrote:

On Wed, Sep 12, 2018 at 1:08 PM, Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Wed, Sep 12, 2018 at 11:53 AM Yuri Wiitala <m...@chromium.org> wrote:

OOC, does base::Bind *need* to allocate the bind state on the heap? Would it cause problems for it to be a data member in the Callback class itself?

That's essentially what my hack CL did. In general, I think it's impossible to do this and maintain a nice API. Callback/Bind has 2 major concepts:

Callback - Generic, type-erased holder. It knows its runtime and nothing else. Corollary: the size is constant

Bind - The main engine for type inference. It analyzes the arguments given to create a correctly sized BindState. This is why it dynamically allocates.

If we want to put the storage on the stack, then we need to be able to, statically, name a type with sufficient storage to hold

the bound parameters which doesn't seem easy.

Crazy idea: what if BindStateStorage was like std::string which does small string optimization:

class BindStateStorage {

 public:

  BindState* get();

 private:

  union {

    char[kSmallBindStateSize] small_state_;

    BindState* big_state_;

  }

  bool is_small_;

};

Yup... That's doable and sorta a specialization of allocate a fixed-sized buffer (with support functions to handle a polymorphic

copy of the opaque type).  This could remove the "auto", but I think the rest of the CallbackRegion API exists. Specifically,

the constraints seem to be:

(a) There must be a holder type (eg, CallbackRegion) that defines the lifetime and storage of the BindState on the stack

(b) This holder type may want to be neither moveable or copyable. There's an argument for moveable, but

     implementation and error complexity will grow, especially with bound move-only types. (This a problem with std::function)

(c) There must be a way to extract a base::Callback<> from that type. Without a user-defined conversion, you must

      do .ToCallback() or similar.

You can't get rid of (a) because that's how we handle storage and lifetime.

You probably don't want to get rid of (c) because otherwise we'd need another "functor" abstraction which is

suboptimal; a single codebase really should aim to have only 1 functor abstraction. (hence the resistance to std::function)

-Albert

[Albert J. Wong (王重傑)]

Given the continued interest I think I'm going to try and iterate on https://chromium-review.googlesource.com/c/chromium/src/+/1220629 and see if I can come up with a real proposal. The current summary of what the impl current does is:

(1) It creates a HolderType "CallbackRegion<>" that stack-allocates a BindState wrapping 

     a lambda (both capture and non-capture). Using template deduction and auto, this type is

     exactly correctly sized.

(2) The CallbackRegion as an accessor to a base::Callback<> of the right type. This can be

     passed to all APIs keeping base::Callback<> as the universal functor abstraction.

(3) The returned base::Callback<> is ref-counted, but the destructor is a no-op.

(4) When CallbackRegion goes out of scope, it has a CHECK() that the base::Callback has

     only 1 reference (itself).

Combining this with a both capturing and non-capturing lambdas we should get:

  (a) Fast allocation. I haven't benched it, but it should be roughly as fast as using a raw lambda.

  (b) dynamic assertion that the base::Callback<> cannot be retained beyond the scope of the holder.

Which addresses both the perf and the safety concerns we've historically had with base::Bind() and

capturing lambdas.

The API looks as follows:

TEST_F(CallbackTest, CallbackRegion) {

  int x = 3;

  auto callback_region = FromFunctor([&x](int a) { return a + x; });

  base::RepeatingCallback<int(int)> cb = callback_region.ToCallback();

  EXPECT_EQ(7, cb.Run(4));

  EXPECT_EQ(8, cb.Run(5));

  x = -3;

  EXPECT_EQ(1, cb.Run(4));

  EXPECT_EQ(2, cb.Run(5));

}

TEST_F(CallbackTest, CallbackRegion_Crash) {

  int x = 3;

  base::RepeatingCallback<int(int)> stored_cb;

  LOG(ERROR) << "This should crash";

  {

    auto callback_region = FromFunctor([&x](int a) { return a + x; });

    stored_cb = callback_region.ToCallback();

  }

}

With a little more work, it could likely be extended to use base::Bind(), but given that we have a region-style

memory management, I'm not certain there's a point. I think a capturing lambda ends up being

good enough...

Thoughts?  Concerns? Objections? Cheers?

-Albert

[Dmitry Skiba]

Hi Albert,

In your proposal, what happens when a callback outlives its region? Is it going to CHECK at runtime?

Also, if we're going to consider capturing lambdas, why not just allow them in a form of non-copyable/non-movable Callback variant? All std::function questions I saw (and asked myself) were about using std::function for local callbacks, which are invoked during a function call, and not copied/stored.

How about this:

void Foo(base::LocalCallback<int(int)>& callback) {

callback.Run(42);

...

}

int x = 3;

base::LocalCallback<int(int)> cb = [&x](int a) { return a + x; };

Foo(cb);

?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALcbsXAOhvZ39Dn7CSno3_UNf%2BVqF%2Bjq%3DgvUS%2BRgq3zRSh87Sg%40mail.gmail.com.

[Albert J. Wong (王重傑)]

On Wed, Sep 12, 2018 at 3:55 PM Dmitry Skiba <dsk...@google.com> wrote:

Hi Albert,

In your proposal, what happens when a callback outlives its region? Is it going to CHECK at runtime?

Yup. Exactly. That's what the unittest shows.

 

Also, if we're going to consider capturing lambdas, why not just allow them in a form of non-copyable/non-movable Callback variant? All std::function questions I saw (and asked myself) were about using std::function for local callbacks, which are invoked during a function call, and not copied/stored.

How about this:

void Foo(base::LocalCallback<int(int)>& callback) {

callback.Run(42);

...

}

int x = 3;

base::LocalCallback<int(int)> cb = [&x](int a) { return a + x; };

Foo(cb);

So this is what I tried first. I think we end up with the same problem which is that what is stores lambda callback inside LocalCallback<>? Specifically

template <typename Sig>

class LocalCallback {

  template <typename Functor>

  LocalCallback(Functor my_lambda) {
    /* What do you do here with my_lambda? How would you declare the field? */
  }

[Vladimir Levin]

On Wed, Sep 12, 2018 at 4:02 PM Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Wed, Sep 12, 2018 at 3:55 PM Dmitry Skiba <dsk...@google.com> wrote:

Hi Albert,

In your proposal, what happens when a callback outlives its region? Is it going to CHECK at runtime?

Yup. Exactly. That's what the unittest shows.

 

Also, if we're going to consider capturing lambdas, why not just allow them in a form of non-copyable/non-movable Callback variant? All std::function questions I saw (and asked myself) were about using std::function for local callbacks, which are invoked during a function call, and not copied/stored.

How about this:

void Foo(base::LocalCallback<int(int)>& callback) {

callback.Run(42);

...

}

int x = 3;

base::LocalCallback<int(int)> cb = [&x](int a) { return a + x; };

Foo(cb);

So this is what I tried first. I think we end up with the same problem which is that what is stores lambda callback inside LocalCallback<>? Specifically

template <typename Sig>

class LocalCallback {

  template <typename Functor>

  LocalCallback(Functor my_lambda) {
    /* What do you do here with my_lambda? How would you declare the field? */
  }

}

Use std::function<Sig> ;) At least you get the safety is this is the only exception

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALcbsXD%2Bf-KsNpzBSfvzuhRLNDnfGj5JNwgAKnGd%2Bjw7qTpBuA%40mail.gmail.com.

[Albert J. Wong (王重傑)]

On Wed, Sep 12, 2018 at 4:13 PM Vladimir Levin <vmp...@chromium.org> wrote:

On Wed, Sep 12, 2018 at 4:02 PM Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Wed, Sep 12, 2018 at 3:55 PM Dmitry Skiba <dsk...@google.com> wrote:

Hi Albert,

In your proposal, what happens when a callback outlives its region? Is it going to CHECK at runtime?

Yup. Exactly. That's what the unittest shows.

 

Also, if we're going to consider capturing lambdas, why not just allow them in a form of non-copyable/non-movable Callback variant? All std::function questions I saw (and asked myself) were about using std::function for local callbacks, which are invoked during a function call, and not copied/stored.

How about this:

void Foo(base::LocalCallback<int(int)>& callback) {

callback.Run(42);

...

}

int x = 3;

base::LocalCallback<int(int)> cb = [&x](int a) { return a + x; };

Foo(cb);

So this is what I tried first. I think we end up with the same problem which is that what is stores lambda callback inside LocalCallback<>? Specifically

template <typename Sig>

class LocalCallback {

  template <typename Functor>

  LocalCallback(Functor my_lambda) {
    /* What do you do here with my_lambda? How would you declare the field? */
  }

}

Use std::function<Sig> ;) At least you get the safety is this is the only exception

Hah. Fair enough. So we could create a non-copyable type that wraps std::function<> (or 

reimplements it enough) to enforce that a lambda isn't stored and then only pass it around

by const ref?

The trade off is such a Functor wouldn't be passable into another API that was base::Callback<>

based should that be interesting.  One could hypothesize, especially in tests, wanting to

create a local captured lambda that you could PostTask() around where you have external

synchronization that ensures all tasks are completed before destruction...

[Vladimir Levin]

I think that would be great.

 

The trade off is such a Functor wouldn't be passable into another API that was base::Callback<>

based should that be interesting.  One could hypothesize, especially in tests, wanting to

create a local captured lambda that you could PostTask() around where you have external

synchronization that ensures all tasks are completed before destruction...

That seems like a reasonable limitation. One could always fallback to using base::Callback/base::Bind

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALcbsXCVAZgQPavp-kfv3UupB38QAZ_w6FK%3DwEQyPfKy0g%3D3LQ%40mail.gmail.com.

[jdoe...@chromium.org]

A few drive by comments:

Why not simply allowing capturing lambdas as well and simply making the struct non-copyable and non-movable?

This could look like this for instance: https://godbolt.org/z/hNIzJL

 

 

The trade off is such a Functor wouldn't be passable into another API that was base::Callback<>

based should that be interesting.  One could hypothesize, especially in tests, wanting to

create a local captured lambda that you could PostTask() around where you have external

synchronization that ensures all tasks are completed before destruction...

That seems like a reasonable limitation. One could always fallback to using base::Callback/base::Bind

Yep. Also we already have base::BindLambdaForTesting() allowing the construction of base::Callbacks from capturing lambdas.

I might be missing something, but why couldn't we try to optimize the regular base::Bind and employ a small buffer optimization in base::BindState?

We  likely would need to modify CallbackBase to store a BindState on the stack, but we should be able to employ the same guarantees that an

OnceCallback should only be invoked once, etc. FWIW, the idea isn't all that crazy, as this is exactly what libc++'s std::function implementation does.

On Tuesday, September 11, 2018 at 9:07:53 PM UTC+2, Daniel Cheng wrote:

On Tue, Sep 11, 2018 at 12:00 PM Daniel Cheng <dch...@chromium.org> wrote:

The guidelines for C++ usage in Blink are the same as the guidelines for C++ usage in Chrome, so std::function is banned in Blink =)

1) It would be useful to understand where the performance difference between base::Callback and std::function are coming from: my guess is base::Callback involves more indirections, since the BindState is always heap-allocated. In contrast, std::function doesn't always require going to the heap.

2) A discussion of whether or not to allow std::function probably belongs on c...@chromium.org (which I've added to this thread).

Daniel

On Tue, Sep 11, 2018 at 11:50 AM Emil A Eklund <e...@chromium.org> wrote:

Hi,

What's our policy on using std::function in Blink? We have a number of
uses of it in blink code already but the Chromium guide explicitly
forbids it [1].

The reason I'm asking is because the recommended alternative,
base::Callback, is *significantly* slower. We're seeing a 5-10%
regression on some of the layout performance tests when using
base::Callback but a slight improvement when using std::function to
replace the same lambda [2] on both high- and low-end devices where
ArabicLineLayout gets ~12% slower with base::Callback [3] and ~1%
faster with std::function [4] .

If the ban applies, what's the recommended way to implement a
synchronous typed callback function? The base::Callback overhead is
somewhat problematic in this case.

Sorry I realize I didn't answer this question. I think the most common patterns (that don't involve std::function) are:

- pass a function pointer

- accept the functor as a template parameter so lambdas can be passed

Small piece of trivia: Non-capturing lambdas actually do implicitly convert to the corresponding function pointers,

as defined by the standard: http://eel.is/c++draft/expr.prim.lambda#closure-7

So if you want to avoid the overhead of a base::Callback, don't need to capture state, and want it to work across

compilation units, accepting a function pointer does seem like a good choice.

 

I'm guessing the latter doesn't work well for layout, but could the former be an option?

Daniel

 

Thanks

1: https://chromium-cpp.appspot.com/#library-blacklist
2: https://chromium-review.googlesource.com/c/chromium/src/+/1208728/4..5
3: https://pinpoint-dot-chromeperf.appspot.com/results2/1395c415640000
4: https://pinpoint-dot-chromeperf.appspot.com/results2/109cf38d640000

--
Emil

-- 
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-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CADu_oUD%3DTqDB%3D0pjK9kwQaj3yhKyrTD7WWCFegO03LfgYWNZrg%40mail.gmail.com.

 

[Daniel Cheng]

That's a possibility, but I think it could still lead to a situation where performance degrades surprisingly in the case when the code changes in a way to make the functor no longer fit in the inline buffer.

(To me, it also seems like it would be kind of nice if there was a difference between callbacks that can run asynchronously vs callbacks that always run synchronously)

I should have been clearer, but when I wrote lambda above, I meant a capturing lambda. I looked at getting rid of current std::function usage, and a number of them aren't possible without larger transformations, due to the use of capturing lambdas.

Daniel

 

 

I'm guessing the latter doesn't work well for layout, but could the former be an option?

Daniel

 

Thanks

1: https://chromium-cpp.appspot.com/#library-blacklist
2: https://chromium-review.googlesource.com/c/chromium/src/+/1208728/4..5
3: https://pinpoint-dot-chromeperf.appspot.com/results2/1395c415640000
4: https://pinpoint-dot-chromeperf.appspot.com/results2/109cf38d640000

--
Emil

-- 
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 post to this group, send email to platform-arc...@chromium.org.

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

 

--

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 post to this group, send email to platform-arc...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/795d165c-8403-40cd-9f01-663357b71bc6%40chromium.org.

[Jeremy Roman]

Lacking anything better, another alternative might be to have the function take an interface pointer and have the caller supply an implementation functor. This throws away some of the syntactic goodness, but has basically the interesting properties here.

If this is a pattern we want to replicate elsewhere, we could consider providing something like LLVM's function ref, whose basic implementation is roughly this (the actual details are somewhat more subtle, but this is the gist):

template <typename R, typename... A> class function_ref {

 public:

  template <typename Callable> function_ref(const Callable& c)

    : func_(&Dispatch<Callable>), ptr_(&c) {}

  A operator()(A... args) const {

    return func_(std::forward<A>(args)...);

  }

 private:

  template <typename Callable>

  static R Dispatch(const void* p, A... args) {

    return (*static_cast<const Callable*>(ptr))(std::forward<A>(args)...);

  }

  using FunctionPointer = R(*)(void*, A...);

  FunctionPointer func_;

  const void* ptr_;

};

This is strictly dumber than std::function, but does the thing that seems to be desired here. It does give virtual dispatch to arbitrary lambdas and does not in any way extend their lifetime.

On Tue, Sep 11, 2018 at 9:07 PM Daniel Cheng <dch...@chromium.org> wrote:

std::function has a number of gotchas that make me hesitant to allow both (at least without better guidelines/recommendations of when std::function is appropriate).

- std::function can only be copied, not moved. If the functor it captures has a significant amount of state, this copy is expensive.

[citation needed] std::function definitely has a move constructor (https://en.cppreference.com/w/cpp/utility/functional/function/function)

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To post to this group, send email to c...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAF3XrKqBpWcchSNWiFPLvPBq6cAFndFxq09NdO0QP-qvbu7eog%40mail.gmail.com.

[Karl Wiberg]

On Fri, Sep 14, 2018 at 5:57 PM Jeremy Roman <jbr...@chromium.org> wrote:

Lacking anything better, another alternative might be to have the function take an interface pointer and have the caller supply an implementation functor. This throws away some of the syntactic goodness, but has basically the interesting properties here.

If this is a pattern we want to replicate elsewhere, we could consider providing something like LLVM's function ref, whose basic implementation is roughly this (the actual details are somewhat more subtle, but this is the gist):

template <typename R, typename... A> class function_ref {

 public:

  template <typename Callable> function_ref(const Callable& c)

    : func_(&Dispatch<Callable>), ptr_(&c) {}

  A operator()(A... args) const {

    return func_(std::forward<A>(args)...);

  }

 private:

  template <typename Callable>

  static R Dispatch(const void* p, A... args) {

    return (*static_cast<const Callable*>(ptr))(std::forward<A>(args)...);

  }

  using FunctionPointer = R(*)(void*, A...);

  FunctionPointer func_;

  const void* ptr_;

};

This is strictly dumber than std::function, but does the thing that seems to be desired here. It does give virtual dispatch to arbitrary lambdas and does not in any way extend their lifetime.

We did this in WebRTC: https://cs.chromium.org/chromium/src/third_party/webrtc/rtc_base/function_view.h

We haven't actually ended up using it as often as I initially thought, but it's very nice to have for the cases where it's the right tool (that is, when you have a non-template function that takes a callable argument that it will only use for the duration of the call).

 

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACuR13f%2B6m0W8QgD7MAvhZ6t5%2BEL63CB1bdAxTEDH0rKK8Cjqg%40mail.gmail.com.

--

Karl Wiberg

Software Engineer

kwi...@google.com

+46 70 696 1024

[Albert J. Wong (王重傑)]

Interesting!

So, this particular variant of type-erasure I think only handles non-capturing callbacks.  If this is all the we want, I think it'd be reasonable to folk it back into base::Callback<> as that's what base::Callback<> is doing as well (see equivalent code here https://cs.chromium.org/chromium/src/base/callback.h?q=Callback&sq=package:chromium&g=0&l=96)

However, for emil's original example, there was a wish to bind as well, but not dynamically allocate. 

I feel like I'm starting to get lost in all the suggestions, so I collated all the ideas into a design proposal here.  Please feel free to add comments or corrections.

At this point, I'm wondering if the best tradeoff is actually still CallbackRegion (probably with a better name...but we can bikeshed later), but with an implicit conversion added...

https://docs.google.com/document/d/17clHO7b4UOvDL7BFfKzU8iFzwx5ozY0DFzoIqAX1Jsk/edit#

(FYI, I've given comment options to the world but that means I couldn't give edit options to chromium.org.  If you want edit perms, just ping me.)

-Albert

[Jeremy Roman]

On Mon, Sep 17, 2018 at 3:50 PM Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Interesting!

So, this particular variant of type-erasure I think only handles non-capturing callbacks.  If this is all the we want, I think it'd be reasonable to folk it back into base::Callback<> as that's what base::Callback<> is doing as well (see equivalent code here https://cs.chromium.org/chromium/src/base/callback.h?q=Callback&sq=package:chromium&g=0&l=96)

However, for emil's original example, there was a wish to bind as well, but not dynamically allocate. 

As commented on the doc, LLVM's function_ref (and also WebRTC's FunctionView) work fine with capturing lambdas and do no extra allocation outside two-word struct.

[Albert J. Wong (王重傑)]

On Mon, Sep 17, 2018 at 1:23 PM Jeremy Roman <jbr...@chromium.org> wrote:

On Mon, Sep 17, 2018 at 3:50 PM Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Interesting!

So, this particular variant of type-erasure I think only handles non-capturing callbacks.  If this is all the we want, I think it'd be reasonable to folk it back into base::Callback<> as that's what base::Callback<> is doing as well (see equivalent code here https://cs.chromium.org/chromium/src/base/callback.h?q=Callback&sq=package:chromium&g=0&l=96)

However, for emil's original example, there was a wish to bind as well, but not dynamically allocate. 

As commented on the doc, LLVM's function_ref (and also WebRTC's FunctionView) work fine with capturing lambdas and do no extra allocation outside two-word struct.

Interesting... you are right. I misread that. But I'm trying to wrap my head around the memory safety guarantees of this.

Is it that it is only ever used as an argument type and never declared on the stack?

-Albert

[Jeremy Roman]

On Mon, Sep 17, 2018 at 4:37 PM Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

On Mon, Sep 17, 2018 at 1:23 PM Jeremy Roman <jbr...@chromium.org> wrote:

On Mon, Sep 17, 2018 at 3:50 PM Albert J. Wong (王重傑) <ajw...@chromium.org> wrote:

Interesting!

So, this particular variant of type-erasure I think only handles non-capturing callbacks.  If this is all the we want, I think it'd be reasonable to folk it back into base::Callback<> as that's what base::Callback<> is doing as well (see equivalent code here https://cs.chromium.org/chromium/src/base/callback.h?q=Callback&sq=package:chromium&g=0&l=96)

However, for emil's original example, there was a wish to bind as well, but not dynamically allocate. 

As commented on the doc, LLVM's function_ref (and also WebRTC's FunctionView) work fine with capturing lambdas and do no extra allocation outside two-word struct.

Interesting... you are right. I misread that. But I'm trying to wrap my head around the memory safety guarantees of this.

Is it that it is only ever used as an argument type and never declared on the stack?

Exactly. function_ref/FV is to lambda as base::StringPiece is to std::string, or base::span is to std::vector. It's only suitable for short-term use, almost exclusively as function parameters. But it's pretty concise and efficient at that.

[Karl Wiberg]

Yes, that's one safe way to use it. Think of it as a raw pointer: very cheap to copy, but you need to arrange for the pointee to stay alive somehow.

If you're familiar with std::span or its cousins, that's an even better analogy.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CALcbsXDEfvWwmeSBWTDeG5M0NA6hHGgquCy3%3DxfdfWR2gBq7PQ%40mail.gmail.com.