Tencent / flare

Flare是广泛投产于腾讯广告后台的现代化C++开发框架,包含了基础库、RPC、各种客户端等。主要特点为易用性强、长尾延迟低。
Other
1.33k stars 200 forks source link

Add Fiber::get_id and use ExitBarrier's address as the fiber ID #151

Closed autopear closed 6 months ago

autopear commented 6 months ago

Similar to std::thread, Fiber should also have a get_id method.

0x804d8000 commented 6 months ago

The change is reasonable, but I'm a little concerned with the way fiber's ID is defined.

Although the address of ExitBarrier is unique for each fiber, it seems unintuitive to use it as fiber's ID as opposed to using the address of FiberEntity. I guess this had something to do why I didn't add this member function to Fiber...

If we're going to add such a member function, I think it's better for us to somehow figure out how to associate fiber's ID with its control block, i.e., FiberEntity.

0x804d8000 commented 6 months ago

On a second though, I think the use of ExitBarrier's address is justified. FiberEntity is associated with fiber's stack, which is a scarce resource. We can't afford to keep fiber's stack held until all instances of its Fibers are gone. Therefore, fiber's stack, as well as FiberEntity, may well be gone long before all Fibers are gone, and using FiberEntity's address is no longer a viable option.

I'm ok with the overall design now.

The CIs are failing, do you mind taking a look at them?

autopear commented 6 months ago

On a second though, I think the use of ExitBarrier's address is justified. FiberEntity is associated with fiber's stack, which is a scarce resource. We can't afford to keep fiber's stack held until all instances of its Fibers are gone. Therefore, fiber's stack, as well as FiberEntity, may well be gone long before all Fibers are gone, and using FiberEntity's address is no longer a viable option.

I'm ok with the overall design now.

The CIs are failing, do you mind taking a look at them?

It's fixed now, but I'm not sure why the bazel build is failing.

0x804d8000 commented 6 months ago

Merged, thanks!

I was meant to take a look at Bazel failure but it was deferred due to my time constraints. It's not related to your changes.