Skip to content
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

standalone-enc: use processes instead of threads for socketpair_one #399

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

gperciva
Copy link
Member

No description provided.

@gperciva
Copy link
Member Author

On bare-metal freebsd 14.0, there's a very slight increase in time by switching from threads to processes. I don't think it's relevant, but I figured I should mention it.

N=11 boxplot

overall

fork

close-up of socketpair_one test

closeup

@gperciva gperciva changed the title standalone-enc: use process instead of threads for socketpair_one standalone-enc: use processes instead of threads for socketpair_one Feb 16, 2024
@cperciva
Copy link
Member

I don't see any explanation of why you want this change?

@gperciva
Copy link
Member Author

Two reasons:

  1. this is closer to the way we'd normally use spiped (i.e. one process from spiped, and at least one other process piping data to it).

  2. this will allow us to add a socketpair_two test (which has separate processes for encryption, decryption, input, and output), with less new code.

    We can't have encryption and decryption in the same process, because they rely on events_spin(), which isn't multi-thread-safe.

I can add this to the commit message for 7ab5011.

@cperciva
Copy link
Member

Makes sense. Yes, please add some text to the relevant commit message.

@gperciva gperciva force-pushed the standalone-1-process branch from 43e3314 to 31c71fa Compare February 20, 2024 20:25
@gperciva
Copy link
Member Author

Updated.

FWIW, this PR changes from pthread_create_blocking_np to fork_func without any "blocking" functionality.

If I add that in -- making sure that the forked functions have started, before leaving pipe_init() -- then the forked version is slightly faster than both previous versions:

sync

gperciva added 3 commits June 27, 2024 09:20
This commit has two benefits:

1. It allows us to add a socketpair_two test (which has separate
   processes for encryption, decryption, input, and output) with less
   new code.

   (We can't have encryption and decryption in the same process, because
   they rely on events_spin(), which isn't thread-safe.)

2. This brings the test closer to the way we'd normally use spiped (i.e.
   one process from spiped, and at least one other process piping data
   to it).
@gperciva gperciva force-pushed the standalone-1-process branch from abec9b2 to cc1bedb Compare June 27, 2024 16:27
@gperciva gperciva marked this pull request as draft June 29, 2024 02:05
@gperciva
Copy link
Member Author

Actually, wait, I want to revisit the number of open file descriptors in the forked processes. I'm not convinced it's wrong, but I'm also not convinced it's right.

@gperciva gperciva marked this pull request as ready for review June 29, 2024 16:14
@gperciva
Copy link
Member Author

Actually, wait, I want to revisit the number of open file descriptors in the forked processes.

Never mind, this is the normal behaviour of fork.

@cperciva cperciva merged commit 2e453db into master Jul 15, 2024
2 checks passed
@gperciva gperciva deleted the standalone-1-process branch July 23, 2024 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants