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

dependency reduction #625

Closed
3 of 4 tasks
spacejam opened this issue Apr 8, 2019 · 9 comments
Closed
3 of 4 tasks

dependency reduction #625

spacejam opened this issue Apr 8, 2019 · 9 comments

Comments

@spacejam
Copy link
Owner

spacejam commented Apr 8, 2019

  • when 1.35 is released: remove futures, use std futures
  • replace hashbrown if it becomes std default
  • replace rayon with simple internal threadpool
  • replace serde requirement
@jeehoonkang
Copy link
Contributor

On rayon: it seems sled depends on rayon for its thread pool. Do you intend to implement a lightweight fork of it and use the fork? I'd also like to ask what's the motivation behind this effort. Rayon thread pool is quite good :)

@spacejam
Copy link
Owner Author

@jeehoonkang mostly I want to be fairly aggressive about keeping dependencies minimized, and rayon is using an ancient version of crossbeam, and I'd like to be able to reduce this duplicated dependency. I'm already implementing threadpool draining on top of rayon, which is already a good chunk of the total complexity I need in an implementation. I don't need a complex threadpool, but the ability to grow when necessary might be another nice thing to have which rayon probably won't implement. Threadpools aren't very complex, and I can remove a large number of the total dependencies in sled by rolling my own.

@Kerollmops
Copy link
Contributor

Kerollmops commented Apr 24, 2019

Hashbrown is now in the std library, you will need to create a type alias and depend on the rustc-hash crate to get the FxHasher type. It will not really reduce the number of dependencies though :)

rust-lang/rust#58623 (comment)

@spacejam
Copy link
Owner Author

one down is still nice :]

@Kerollmops
Copy link
Contributor

Kerollmops commented Apr 24, 2019

You will replace hashbrown by rustc-hash actually :) ok, you will no more need to compile hashbrown, good point!

EDIT: you will need to compile hashbrown, in fact.

@kanru
Copy link

kanru commented May 4, 2019

It looks like the current Futures usage is quite simple. The receiving side is always a blocking wait() on the result (one in pagecache/src/segment.rs and one in subscription.rs). If I understand correctly, they can be replaced by a mpsc async channel.

@spacejam
Copy link
Owner Author

spacejam commented Jul 8, 2019

@kanru in the near term it's not so hard to just replace it with a mutex with a thin layer on top that sort of acts like a oneshot channel. Since the futures support in std doesn't really work for the sled use case (we don't want to give up control to a specific scheduler that makes a static decision about latency vs throughput)

@spacejam
Copy link
Owner Author

rayon and lazy_static are now replaced by simple implementations that don't trigger lots of TSAN false positives

@spacejam
Copy link
Owner Author

For now, serde will stay, as we don't need to deserialize anything during recovery anymore (log message headers provide everything we need now)

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

No branches or pull requests

4 participants