-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
ARROW-3958: [Plasma] Reduce number of IPCs #3124
Changes from 11 commits
24beb27
5f09199
e5ccbba
cfff7e3
2887b17
502aeda
f60dcbe
0d57282
71c4c5c
af150c1
a038404
f899f45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,11 +34,6 @@ using arrow::Status; | |
|
||
namespace plasma { | ||
|
||
/// We keep a queue of unreleased objects cached in the client until we start | ||
/// sending release requests to the store. This is to avoid frequently mapping | ||
/// and unmapping objects and evicting data from processor caches. | ||
constexpr int64_t kPlasmaDefaultReleaseDelay = 64; | ||
|
||
/// Object buffer data structure. | ||
struct ObjectBuffer { | ||
/// The data buffer. | ||
|
@@ -62,13 +57,12 @@ class ARROW_EXPORT PlasmaClient { | |
/// \param manager_socket_name The name of the UNIX domain socket to use to | ||
/// connect to the local Plasma manager. If this is "", then this | ||
/// function will not connect to a manager. | ||
/// \param release_delay Number of released objects that are kept around | ||
/// and not evicted to avoid too many munmaps. | ||
/// \param release_delay Deprecated (not used). | ||
/// \param num_retries number of attempts to connect to IPC socket, default 50 | ||
/// \return The return status. | ||
Status Connect(const std::string& store_socket_name, | ||
const std::string& manager_socket_name, | ||
int release_delay = kPlasmaDefaultReleaseDelay, int num_retries = -1); | ||
const std::string& manager_socket_name, int release_delay = 0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Want to just get rid of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the very least, can we remove it from any documentation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
int num_retries = -1); | ||
|
||
/// Create an object in the Plasma Store. Any metadata for this object must be | ||
/// be passed in when the object is created. | ||
|
@@ -354,10 +348,6 @@ class ARROW_EXPORT PlasmaClient { | |
FRIEND_TEST(TestPlasmaStore, LegacyGetTest); | ||
FRIEND_TEST(TestPlasmaStore, AbortTest); | ||
|
||
/// This is a helper method that flushes all pending release calls to the | ||
/// store. | ||
Status FlushReleaseHistory(); | ||
|
||
bool IsInUse(const ObjectID& object_id); | ||
|
||
class ARROW_NO_EXPORT Impl; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file descriptor is not send any more if it had already been send, so we shouldn't close it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we close all the file descriptors in the PlasmaClient destructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they have already been closed by the first LookupOrMmap call (see the close call below), see also comment below