Skip to content

add crossbeam scoped thread #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 6, 2020
Merged

add crossbeam scoped thread #48

merged 2 commits into from
Aug 6, 2020

Conversation

leeopop
Copy link
Contributor

@leeopop leeopop commented Aug 5, 2020

No description provided.

@leeopop leeopop requested a review from jeehoonkang August 5, 2020 02:22
@ANLAB-Support ANLAB-Support force-pushed the feature-crossbeam-thread branch from 906dc5c to 18cb1d4 Compare August 5, 2020 02:26
@leeopop
Copy link
Contributor Author

leeopop commented Aug 5, 2020

bors try

ghost pushed a commit that referenced this pull request Aug 5, 2020
@leeopop
Copy link
Contributor Author

leeopop commented Aug 5, 2020

bors retry

@ghost
Copy link

ghost commented Aug 5, 2020

try

Already running a review

@leeopop
Copy link
Contributor Author

leeopop commented Aug 5, 2020

bors try-

@leeopop
Copy link
Contributor Author

leeopop commented Aug 5, 2020

bors try

ghost pushed a commit that referenced this pull request Aug 5, 2020
@ghost
Copy link

ghost commented Aug 5, 2020

try

Build succeeded:

dpdk/src/eal.rs Outdated

/// Launch a thread pined to this core.
/// TODO: change it to crossbeam's `spawn` signature when we start to use crossbeam.
pub fn launch_scoped<'scope, 'env, F, T>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lifetime이 좀 복잡해보이는데 왜 두개가 필요한가요?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그리고 전통대로lifetime name은 한글자로 합시다.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이게 launch를 대체하는건 어떤가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

scope 를 외부에서 공급받는 형태라서 그런 것 같습니다.

  1. 넵. 저는 큰 선호가 없습니다만 crossbeam 에서 쓰던 convention을 그대로 가져오는게 좋지 않나 싶어서 우선 그쪽 naming을 차용했습니다.

  2. 고민을 해 보았습니다만 (std vs 3rdparty) 애초에 Rust std 의 thread launch 모델이 좋은 것 같지 않아서 아예 crossbeam 으로 대체하는게 좋겠습니다. 넵 좋습니다.

@leeopop leeopop requested a review from jeehoonkang August 6, 2020 00:59
@leeopop leeopop merged commit 349a03b into master Aug 6, 2020
@ghost ghost deleted the feature-crossbeam-thread branch August 6, 2020 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants