Skip to content

Commit

Permalink
A small bit of coverage cleanup (#38)
Browse files Browse the repository at this point in the history
* Small fix for that weird Win32 VM line coverage

* More better

* Switch to a fast return layout

* And the partials reveal themselves

* I have a sneaking suspicion

* And the fix
  • Loading branch information
Cyberboss committed Nov 29, 2016
1 parent 9165c07 commit de4aeee
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 16 deletions.
33 changes: 22 additions & 11 deletions Engine/Platform/System/Win32/CYBWin32VirtualMemory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ unsigned int CYB::Platform::System::Implementation::VirtualMemory::SystemPageSiz
}

void* CYB::Platform::System::Implementation::VirtualMemory::PageAlignedUpperBound(void* AMemory, const unsigned int APageSize) noexcept {
if ((reinterpret_cast<unsigned long long>(AMemory) % APageSize) == 0)
return AMemory;
return reinterpret_cast<void*>(reinterpret_cast<unsigned long long>(AMemory) + (APageSize - (reinterpret_cast<unsigned long long>(AMemory) % APageSize)));
}

Expand Down Expand Up @@ -68,16 +70,25 @@ void CYB::Platform::System::VirtualMemory::Discard(void* const AMemory, const un
API::Assert::LessThanOrEqual(static_cast<byte*>(FReservation), static_cast<byte*>(AMemory));
API::Assert::LessThanOrEqual(static_cast<byte*>(AMemory), static_cast<byte*>(FReservation) + FCommitSize);
API::Assert::LessThanOrEqual(static_cast<byte*>(AMemory) + ANumBytes, static_cast<byte*>(FReservation) + FCommitSize);

if (!Core().FModuleManager.FK32Extended.Loaded<Modules::Kernel32Extended::DiscardVirtualMemory>())
return;

const auto PageSize(Implementation::VirtualMemory::SystemPageSize());
if (ANumBytes >= PageSize) {
auto const AlignedMemory(Implementation::VirtualMemory::PageAlignedUpperBound(AMemory, PageSize));
const auto Difference(reinterpret_cast<unsigned long long>(AlignedMemory) - reinterpret_cast<unsigned long long>(AMemory));
if (Difference < ANumBytes) {
const auto BytesAvailableToDiscard(ANumBytes - Difference);
const auto TrueDiscardSize(BytesAvailableToDiscard - (BytesAvailableToDiscard % PageSize));
if(TrueDiscardSize >= PageSize
&& Core().FModuleManager.FK32Extended.Loaded<Modules::Kernel32Extended::DiscardVirtualMemory>())
Core().FModuleManager.FK32Extended.Call<Modules::Kernel32Extended::DiscardVirtualMemory>(AlignedMemory, TrueDiscardSize);
}
}
if (ANumBytes < PageSize)
return;

auto const AlignedMemory(Implementation::VirtualMemory::PageAlignedUpperBound(AMemory, PageSize));
const auto Difference(reinterpret_cast<unsigned long long>(AlignedMemory) - reinterpret_cast<unsigned long long>(AMemory));

//Difference is inherently < PageSize

const auto BytesAvailableToDiscard(ANumBytes - Difference);

if (BytesAvailableToDiscard < PageSize)
return;

const auto TrueDiscardSize(BytesAvailableToDiscard - (BytesAvailableToDiscard % PageSize));

Core().FModuleManager.FK32Extended.Call<Modules::Kernel32Extended::DiscardVirtualMemory>(AlignedMemory, TrueDiscardSize);
}
2 changes: 1 addition & 1 deletion Engine/Platform/System/Win32/CYBWin32VirtualMemory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace CYB {
*/
static unsigned int SystemPageSize(void) noexcept;
/*!
@brief Get the base address of a page given an address
@brief Get the base address of a page on or after a given address
@param AMemory The address to reference
@param APageSize The system page size
@return The base address of the page @p AMemory lies on
Expand Down
40 changes: 36 additions & 4 deletions Tests/Platform/System/VirtualMemory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,51 @@ SCENARIO("VirtualMemory reservation protection levels can be changed", "[Platfor
}
}
}
REDIRECTED_FUNCTION(BadDiscardVirtualMemory, void* const, const unsigned long long) {
return 0;
}
SCENARIO("VirtualMemory can be discarded and reused","[Platform][System][VirtualMemory][Unit]") {
ModuleDependancy<CYB::API::Platform::Identifier::WINDOWS, CYB::Platform::Modules::AMKernel32> K32(CYB::Core().FModuleManager.FK32);
ModuleDependancy<CYB::API::Platform::Identifier::WINDOWS, CYB::Platform::Modules::AMKernel32Extended> K32E(CYB::Core().FModuleManager.FK32Extended);
ModuleDependancy<CYB::API::Platform::POSIX, CYB::Platform::Modules::AMLibC> LibC(CYB::Core().FModuleManager.FC);
#ifdef TARGET_OS_WINDOWS
//For testing purposes, assume it's not there
auto Thing(K32E.Redirect<CYB::Platform::Modules::Kernel32Extended::DiscardVirtualMemory, BadDiscardVirtualMemory>());
#endif
GIVEN("A standard reservation and commit which has some data written to it") {
CYB::Platform::System::VirtualMemory Reservation(1000000);
Reservation.Commit(500000);
*static_cast<unsigned long long*>(Reservation()) = 1234;
WHEN("The memory is discarded") {
REQUIRE_NOTHROW(Reservation.Discard(Reservation(), 500000));
THEN("No errors occur and pages can be reused but data may differ") {
CHECK_NOTHROW(*static_cast<unsigned long long*>(Reservation()) = 5678);
}
auto DiscardSize(250000U);
auto DiscardPoint(Reservation());
const auto Then([&]() {
REQUIRE_NOTHROW(Reservation.Discard(DiscardPoint, DiscardSize));
THEN("No errors occur and pages can be reused but data may differ") {
CHECK_NOTHROW(*static_cast<unsigned long long*>(Reservation()) = 5678);
}
});
Then();
AND_WHEN("The discard size is low") {
DiscardSize = 12;
Then();
}
AND_WHEN("The discard point is close the end of the a page") {
DiscardPoint = static_cast<char*>(DiscardPoint) + 4090;
Then();
}
AND_WHEN("The discard size is okay but crosses a page boundary") {
DiscardSize = 5000;
DiscardPoint = static_cast<char*>(DiscardPoint) + 1500;
Then();
}
#ifdef TARGET_OS_WINDOWS
AND_WHEN("The discard function does not exist") {
using RedirectType = CallRedirect<CYB::Platform::Modules::AMKernel32Extended, CYB::Platform::Modules::Kernel32Extended::DiscardVirtualMemory>;
RedirectType DoIt(CYB::Core().FModuleManager.FK32Extended, static_cast<RedirectType::FCallable*>(nullptr));
Then();
}
#endif
}
}
}
Expand Down

0 comments on commit de4aeee

Please sign in to comment.