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

Rework the listDirectory implementation #242

Merged
merged 4 commits into from
Jan 12, 2021
Merged

Conversation

s-ludwig
Copy link
Member

  • Directly uses OS facilities instead of Phobos to avoid string processing overhead and to enable fast skipping of non-directories
  • Introduces a DirectoryListMode, similar to SpanMode
  • Uses low-overhead channels to reduce the communication overhead between the calling thread and the worker thread that calls the OS
  • Adds FileInfo.path to properly support the new recursive directory iteration schemes

Note that this should only be merged once #241 is merged and this is rebased.

@s-ludwig s-ludwig force-pushed the rework_list_directory branch 2 times, most recently from 1524395 to 30bb993 Compare January 12, 2021 09:27
Sometimes crashed on macos when the logger instance was reaped at runtime shutdown time.
Adds a low-overhead mode to Channel!T that causes the buffer to be fully processed before notifying waiting peers instead of notifying immediately once data/space is available. This heavily reduces the overhead of cross-task/thread notifications at the expense of introducing processing latency and requiring a call to close() to guarantee that all data has been processed.
- Directly uses OS facilities instead of Phobos to avoid string processing overhead and to enable fast skipping of non-directories
- Introduces a DirectoryListMode, similar to SpanMode
- Uses low-overhead channels to reduce the communication overhead between the calling thread and the worker thread that calls the OS
- Adds FileInfo.path to properly support the new recursive directory iteration schemes
@l-kramer l-kramer merged commit 0b6aa0d into master Jan 12, 2021
@s-ludwig s-ludwig deleted the rework_list_directory branch January 12, 2021 13:34
@Geod24
Copy link
Contributor

Geod24 commented Jan 17, 2021

This broke Vibe.d on Musl. First, because AT_SYMLINK_NOFOLLOW was not defined in druntime (bug that's on me), and second, because st_mtimensec and st_ctimensec are not POSIX, they are extensions.

@s-ludwig
Copy link
Member Author

Do you want to go ahead and add a musl based CI job? Granted, not much is happening at the OS interface these days, but since I don't personally use it, that's the only sane way to avoid it breaking.

@s-ludwig
Copy link
Member Author

I just had a look and it would really be nice to unify the Druntime declarations across architectures as far as possible (although that is obviously not an immediate fix). Most systems are actually binary compatible for the time fields, but definitions range from timespec over just time_t to time_t+nsecs with either underscores prefixed or not.

@s-ludwig
Copy link
Member Author

Fix: #249

@Geod24
Copy link
Contributor

Geod24 commented Jan 18, 2021

Do you want to go ahead and add a musl based CI job? Granted, not much is happening at the OS interface these days, but since I don't personally use it, that's the only sane way to avoid it breaking.

I tried that a while ago with Vibe.d and dub, but had some issues, can't remember why. Will take another look.

@Geod24
Copy link
Contributor

Geod24 commented Jan 18, 2021

I just had a look and it would really be nice to unify the Druntime declarations across architectures as far as possible (although that is obviously not an immediate fix)

It would be nice if we could just parse the system headers.

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.

3 participants