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

Add relocatable pointers #20

Merged

Conversation

marthtz
Copy link
Contributor

@marthtz marthtz commented Dec 12, 2019

Fixes #19

Signed-off-by: Hintz Martin (CC-AD/ESW1) [email protected]

Signed-off-by: Hintz Martin (CC-AD/ESW1) <[email protected]>
Comment on lines +60 to +61
// we let the OS decide where to map the shm segments
constexpr void* BASE_ADDRESS_HINT{nullptr};
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we save some pointer arithmetic by giving the operating system a hint that will mostly work for all applications? I could imagine the performance might be slightly better if there is no offset.

@elBoberido @elfenpiff Are there any performance measurements w/ and w/o relocatable pointer?

Comment on lines +40 to +41
// we let the OS decide where to map the shm segments
constexpr void* BASE_ADDRESS_HINT{nullptr};
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here


/// @todo this is a temporary dummy variable to keep the size of the ChunkHeader at 64 byte for compatibility
/// reasons
void* m_payloadDummy{nullptr};
Copy link
Contributor

Choose a reason for hiding this comment

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

m_payloadPadding?

Comment on lines +29 to 31
/// @todo since we are now fully relocatable, this can be removed
uintptr_t m_sharedMemoryBaseAddressOffset = 0;
bool m_verifySharedMemoryPlacement = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you create a follow-up issue for this?

Comment on lines +105 to 106
/// @todo the best guess mapping address as long as we do not have introduced relative pointers
defaultConfig.roudi.m_sharedMemoryBaseAddressOffset = 0x3E80000000ull;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please either remove it or use it as a hint.

Comment on lines +34 to +35
// we let the OS decide where to map the shm segments
constexpr void* BASE_ADDRESS_HINT{nullptr};
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@budrus budrus merged commit 77c84ae into eclipse-iceoryx:master Dec 13, 2019
@marthtz marthtz deleted the iox-#19-add-relocatable-pointers branch December 18, 2020 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add relocatable pointers
3 participants