From 8c8ec21b061a72e729d176e78f756a5da721d66a Mon Sep 17 00:00:00 2001 From: Henri Casanova Date: Wed, 16 Oct 2024 22:59:56 -1000 Subject: [PATCH] bugs-- --- include/wrench/services/Service.h | 2 +- .../compute/cloud/CloudComputeService.h | 1 - .../action_executor/ActionExecutor.h | 2 +- .../wrench/services/storage/StorageService.h | 6 +++++ .../services/storage/StorageServiceMessage.h | 2 +- include/wrench/simulation/Simulation.h | 2 +- src/wrench/services/Service.cpp | 11 ++------ .../compute/cloud/CloudComputeService.cpp | 26 +++++++++---------- .../ActionExecutionService.cpp | 8 +++--- .../action_executor/ActionExecutor.cpp | 2 +- .../services/storage/StorageService.cpp | 4 +-- .../compound/CompoundStorageService.cpp | 4 +-- .../simple/SimpleStorageServiceBufferized.cpp | 6 ++--- .../FileTransferThread.cpp | 23 ++++++++-------- .../StorageServiceLinkFailuresTest.cpp | 11 +++----- .../SimulationTimestampFileCopyTest.cpp | 2 +- test/workflow/WorkflowTaskTest.cpp | 6 ++--- 17 files changed, 56 insertions(+), 62 deletions(-) diff --git a/include/wrench/services/Service.h b/include/wrench/services/Service.h index 8015e299f3..af8e4a2178 100644 --- a/include/wrench/services/Service.h +++ b/include/wrench/services/Service.h @@ -114,7 +114,7 @@ namespace wrench { const WRENCH_PROPERTY_COLLECTION_TYPE &overriden_property_values); // MessagePayload stuff - void setMessagePayload(WRENCH_MESSAGEPAYLOAD_TYPE, double); + void setMessagePayload(WRENCH_MESSAGEPAYLOAD_TYPE, sg_size_t); void setMessagePayloads(const WRENCH_MESSAGE_PAYLOAD_COLLECTION_TYPE &default_messagepayload_values, const WRENCH_MESSAGE_PAYLOAD_COLLECTION_TYPE &overriden_messagepayload_values); diff --git a/include/wrench/services/compute/cloud/CloudComputeService.h b/include/wrench/services/compute/cloud/CloudComputeService.h index 47a1f70e3a..2497add4b9 100644 --- a/include/wrench/services/compute/cloud/CloudComputeService.h +++ b/include/wrench/services/compute/cloud/CloudComputeService.h @@ -111,7 +111,6 @@ namespace wrench { virtual bool isVMDown(const std::string &vm_name); - // std::vector getExecutionHosts(); /***********************/ diff --git a/include/wrench/services/helper_services/action_executor/ActionExecutor.h b/include/wrench/services/helper_services/action_executor/ActionExecutor.h index 2fdfa2a558..b9359e5dc8 100755 --- a/include/wrench/services/helper_services/action_executor/ActionExecutor.h +++ b/include/wrench/services/helper_services/action_executor/ActionExecutor.h @@ -33,7 +33,7 @@ namespace wrench { public: unsigned long getNumCoresAllocated() const; - double getMemoryAllocated() const; + sg_size_t getMemoryAllocated() const; double getThreadCreationOverhead() const; std::shared_ptr getAction(); diff --git a/include/wrench/services/storage/StorageService.h b/include/wrench/services/storage/StorageService.h index d3eb59b34e..08e89e4dcd 100755 --- a/include/wrench/services/storage/StorageService.h +++ b/include/wrench/services/storage/StorageService.h @@ -197,6 +197,7 @@ namespace wrench { } location->getStorageService()->writeFile(location); } + /** * @brief Write a file at the storage service (incurs simulated overheads) * @param file a file @@ -204,6 +205,7 @@ namespace wrench { void writeFile(const std::shared_ptr &file) { this->writeFile(wrench::FileLocation::LOCATION(this->getSharedPtr(), file)); } + /** * @brief Write a file at the storage service (incurs simulated overheads) * @param file a file @@ -212,6 +214,7 @@ namespace wrench { virtual void writeFile(const std::shared_ptr &file, const std::string &path) { this->writeFile(wrench::FileLocation::LOCATION(this->getSharedPtr(), FileLocation::sanitizePath(path), file)); } + /** * @brief Write a file at the storage service (incurs simulated overheads) * @param location a location @@ -235,6 +238,7 @@ namespace wrench { } return location->getStorageService()->hasFile(location); } + /** * @brief Determines whether a file is present at the storage service (in zero simulated time) * @param file: a file @@ -243,6 +247,7 @@ namespace wrench { bool hasFile(const std::shared_ptr &file) { return this->hasFile(wrench::FileLocation::LOCATION(this->getSharedPtr(), file)); } + /** * @brief Determines whether a file is present at the storage service (in zero simulated time) * @param file: a file @@ -252,6 +257,7 @@ namespace wrench { virtual bool hasFile(const std::shared_ptr &file, const std::string &path) { return this->hasFile(wrench::FileLocation::LOCATION(this->getSharedPtr(), FileLocation::sanitizePath(path), file)); } + /** * @brief Determines whether a file is present at the storage service (in zero simulated time) * @param location: a location diff --git a/include/wrench/services/storage/StorageServiceMessage.h b/include/wrench/services/storage/StorageServiceMessage.h index ec332df746..1bc9916434 100755 --- a/include/wrench/services/storage/StorageServiceMessage.h +++ b/include/wrench/services/storage/StorageServiceMessage.h @@ -283,7 +283,7 @@ namespace wrench { * * @param location: the file location **/ - explicit StorageServiceAckMessage(std::shared_ptr location) : StorageServiceMessage(0), location(std::move(location)) {} + explicit StorageServiceAckMessage(std::shared_ptr location) : StorageServiceMessage(S4U_CommPort::default_control_message_size), location(std::move(location)) {} /** @brief The location */ std::shared_ptr location; diff --git a/include/wrench/simulation/Simulation.h b/include/wrench/simulation/Simulation.h index 802d5d00e4..657948cfeb 100755 --- a/include/wrench/simulation/Simulation.h +++ b/include/wrench/simulation/Simulation.h @@ -67,7 +67,7 @@ namespace wrench { void init(int *, char **); static bool isInitialized(); - bool isRunning() const; + [[nodiscard]] bool isRunning() const; void instantiatePlatform(const std::string &); diff --git a/src/wrench/services/Service.cpp b/src/wrench/services/Service.cpp index 096303a868..1c8c4c11d8 100644 --- a/src/wrench/services/Service.cpp +++ b/src/wrench/services/Service.cpp @@ -65,17 +65,10 @@ namespace wrench { /** * @brief Set a message payload of the Service - * @param messagepayload: the message payload (which must a a string representation of a >=0 double) + * @param messagepayload: the message payload (which must be a string representation of a >=0 sg_size_t) * @param value: the message payload value */ - void Service::setMessagePayload(WRENCH_MESSAGEPAYLOAD_TYPE messagepayload, double value) { - // Check that the value is a >=0 double - - if (value < 0) { - throw std::invalid_argument( - "Service::setMessagePayload(): Invalid message payload value " + ServiceMessagePayload::translatePayloadType(messagepayload) + ": " + - std::to_string(value)); - } + void Service::setMessagePayload(WRENCH_MESSAGEPAYLOAD_TYPE messagepayload, sg_size_t value) { this->messagepayload_list[messagepayload] = value; } diff --git a/src/wrench/services/compute/cloud/CloudComputeService.cpp b/src/wrench/services/compute/cloud/CloudComputeService.cpp index f0e3f081c4..fb05d49bc1 100644 --- a/src/wrench/services/compute/cloud/CloudComputeService.cpp +++ b/src/wrench/services/compute/cloud/CloudComputeService.cpp @@ -703,9 +703,9 @@ namespace wrench { std::sort(possible_hosts.begin(), possible_hosts.end(), [](std::string const &a, std::string const &b) { unsigned long a_num_cores = Simulation::getHostNumCores(a); - double a_ram = Simulation::getHostMemoryCapacity(a); + sg_size_t a_ram = Simulation::getHostMemoryCapacity(a); unsigned long b_num_cores = Simulation::getHostNumCores(b); - double b_ram = Simulation::getHostMemoryCapacity(b); + sg_size_t b_ram = Simulation::getHostMemoryCapacity(b); if (a_ram != b_ram) { return a_ram < b_ram; @@ -721,9 +721,9 @@ namespace wrench { std::sort(possible_hosts.begin(), possible_hosts.end(), [](std::string const &a, std::string const &b) { unsigned long a_num_cores = Simulation::getHostNumCores(a); - double a_ram = Simulation::getHostMemoryCapacity(a); + sg_size_t a_ram = Simulation::getHostMemoryCapacity(a); unsigned long b_num_cores = Simulation::getHostNumCores(b); - double b_ram = Simulation::getHostMemoryCapacity(b); + sg_size_t b_ram = Simulation::getHostMemoryCapacity(b); if (a_num_cores != b_num_cores) { return a_num_cores < b_num_cores; @@ -991,13 +991,13 @@ namespace wrench { } else if (key == "ram_capacities") { for (auto &host: this->execution_hosts) { - double total_ram = Simulation::getHostMemoryCapacity(host); - dict.insert(std::make_pair(host, total_ram)); + sg_size_t total_ram = Simulation::getHostMemoryCapacity(host); + dict.insert(std::make_pair(host, (double)total_ram)); } } else if (key == "ram_availabilities") { for (auto &host: this->execution_hosts) { - double total_ram = Simulation::getHostMemoryCapacity(host); + sg_size_t total_ram = Simulation::getHostMemoryCapacity(host); // Available RAM sg_size_t ram_allocated_to_vms = 0; for (auto &vm_pair: this->vm_list) { @@ -1007,7 +1007,7 @@ namespace wrench { ram_allocated_to_vms += actual_vm->getMemory(); } } - dict.insert(std::make_pair(host, total_ram - ram_allocated_to_vms)); + dict.insert(std::make_pair(host, (double)total_ram - (double)ram_allocated_to_vms)); } } else { @@ -1112,11 +1112,11 @@ namespace wrench { return; } auto vm = std::get<0>(this->vm_list[vm_name]); - unsigned long used_cores = vm->getNumCores(); - double used_ram = vm->getMemory(); - std::string pm_name = vm->getPhysicalHostname(); - // WRENCH_INFO("GOT A DEATH NOTIFICATION: %s %ld %lf (exit_code = %d)", - // pm_name.c_str(), used_cores, used_ram, exit_code); +// unsigned long used_cores = vm->getNumCores(); +// sg_size_t used_ram = vm->getMemory(); +// std::string pm_name = vm->getPhysicalHostname(); +// WRENCH_INFO("GOT A DEATH NOTIFICATION: %s %ld %lf (exit_code = %d)", +// pm_name.c_str(), used_cores, used_ram, exit_code); if (vm->getState() != S4U_VirtualMachine::State::DOWN) { vm->shutdown(); diff --git a/src/wrench/services/helper_services/action_execution_service/ActionExecutionService.cpp b/src/wrench/services/helper_services/action_execution_service/ActionExecutionService.cpp index 9b98779b08..fbcf4f2157 100644 --- a/src/wrench/services/helper_services/action_execution_service/ActionExecutionService.cpp +++ b/src/wrench/services/helper_services/action_execution_service/ActionExecutionService.cpp @@ -226,8 +226,8 @@ namespace wrench { std::to_string(requested_cores) + " are requested"); } - double requested_ram = std::get<1>(host.second); - double available_ram = S4U_Simulation::getHostMemoryCapacity(host.first); + sg_size_t requested_ram = std::get<1>(host.second); + sg_size_t available_ram = S4U_Simulation::getHostMemoryCapacity(host.first); if (requested_ram < 0) { throw std::invalid_argument( "ActionExecutionService::ActionExecutionService(): requested RAM should be non-negative"); @@ -334,7 +334,7 @@ namespace wrench { // Compute possible hosts std::set possible_hosts; simgrid::s4u::Host *new_host_to_avoid = nullptr; - double new_host_to_avoid_ram_capacity = 0; + sg_size_t new_host_to_avoid_ram_capacity = 0; for (auto const &r: this->compute_resources) { // If there is a required host, then don't even look at others @@ -442,7 +442,7 @@ namespace wrench { std::string picked_host; simgrid::s4u::Host *target_host = nullptr; unsigned long target_num_cores; - double required_ram; + sg_size_t required_ram; std::tuple allocation = pickAllocation(action, diff --git a/src/wrench/services/helper_services/action_executor/ActionExecutor.cpp b/src/wrench/services/helper_services/action_executor/ActionExecutor.cpp index 015db7f3dc..bb86f35f98 100644 --- a/src/wrench/services/helper_services/action_executor/ActionExecutor.cpp +++ b/src/wrench/services/helper_services/action_executor/ActionExecutor.cpp @@ -185,7 +185,7 @@ namespace wrench { * @brief Return the action executor's allocated RAM * @return a number of bytes */ - double ActionExecutor::getMemoryAllocated() const { + sg_size_t ActionExecutor::getMemoryAllocated() const { return this->ram_footprint; } diff --git a/src/wrench/services/storage/StorageService.cpp b/src/wrench/services/storage/StorageService.cpp index e378f9697a..8ccc08eb2a 100755 --- a/src/wrench/services/storage/StorageService.cpp +++ b/src/wrench/services/storage/StorageService.cpp @@ -143,8 +143,8 @@ namespace wrench { auto file = location->getFile(); for (auto const &dwmb: msg->data_write_commport_and_bytes) { // Bufferized - double remaining = dwmb.second; - while (remaining - buffer_size > DBL_EPSILON) { + sg_size_t remaining = dwmb.second; + while (remaining > buffer_size) { dwmb.first->putMessage( new StorageServiceFileContentChunkMessage( diff --git a/src/wrench/services/storage/compound/CompoundStorageService.cpp b/src/wrench/services/storage/compound/CompoundStorageService.cpp index b4e9fb433e..060c913219 100644 --- a/src/wrench/services/storage/compound/CompoundStorageService.cpp +++ b/src/wrench/services/storage/compound/CompoundStorageService.cpp @@ -191,9 +191,9 @@ namespace wrench { // Stripping case if (file_size > this->max_chunk_size && this->internal_stripping) { WRENCH_INFO("CSS::processStorageSelectionMessage(): Stripping file before sending to allocator"); - double remaining = file_size; + sg_size_t remaining = file_size; auto part_id = 0; - while (remaining - this->max_chunk_size > DBL_EPSILON) { + while (remaining > this->max_chunk_size) { std::string part_file_name = file_name + "_stripe_" + std::to_string(part_id); std::shared_ptr part_file = wrench::Simulation::getFileByIDOrNull(part_file_name); if (not part_file) { diff --git a/src/wrench/services/storage/simple/SimpleStorageServiceBufferized.cpp b/src/wrench/services/storage/simple/SimpleStorageServiceBufferized.cpp index 370b40340f..bbac5da227 100644 --- a/src/wrench/services/storage/simple/SimpleStorageServiceBufferized.cpp +++ b/src/wrench/services/storage/simple/SimpleStorageServiceBufferized.cpp @@ -21,6 +21,7 @@ #include #include #include +#include //#include namespace sgfs = simgrid::fsmod; @@ -495,10 +496,9 @@ namespace wrench { // Send back the relevant ack if this was a read if (ftt->dst_location == nullptr) { - // WRENCH_INFO("Sending back an ack for a successful file read"); - ftt->answer_commport_if_read->dputMessage(new StorageServiceAckMessage(ftt->src_location)); + ftt->answer_commport_if_read->dputMessage(new StorageServiceAckMessage(ftt->src_location)); } else if (ftt->src_location == nullptr) { - WRENCH_INFO("File %s stored", ftt->dst_location->getFile()->getID().c_str()); + ftt->answer_commport_if_write->dputMessage(new StorageServiceAckMessage(ftt->dst_location)); } else { if (ftt->dst_location->getStorageService() == shared_from_this()) { diff --git a/src/wrench/services/storage/storage_helper_classes/FileTransferThread.cpp b/src/wrench/services/storage/storage_helper_classes/FileTransferThread.cpp index c640b4ca00..9e99249e52 100644 --- a/src/wrench/services/storage/storage_helper_classes/FileTransferThread.cpp +++ b/src/wrench/services/storage/storage_helper_classes/FileTransferThread.cpp @@ -280,7 +280,7 @@ namespace wrench { S4U_CommPort *commport, const std::shared_ptr &location) { /** Ideal Fluid model buffer size */ - if (this->buffer_size < DBL_EPSILON) { + if (this->buffer_size == 0) { throw std::runtime_error( "FileTransferThread::receiveFileFromNetwork(): Zero buffer size not implemented yet"); @@ -399,7 +399,7 @@ namespace wrench { sg_size_t num_bytes, S4U_CommPort *commport) { /** Ideal Fluid model buffer size */ - if (this->buffer_size < DBL_EPSILON) { + if (this->buffer_size == 0) { throw std::runtime_error( "FileTransferThread::sendLocalFileToNetwork(): Zero buffer size not implemented yet"); @@ -408,7 +408,8 @@ namespace wrench { /** Non-zero buffer size */ std::shared_ptr req = nullptr; // Sending a zero-byte f is really sending a 1-byte f - double remaining = std::max(1, num_bytes); + sg_size_t remaining = std::max(1, num_bytes); + #ifdef PAGE_CACHE_SIMULATION if (Simulation::isPageCachingEnabled()) { @@ -416,8 +417,8 @@ namespace wrench { } #endif - while (remaining > DBL_EPSILON) { - double chunk_size = std::min(this->buffer_size, remaining); + while (remaining > 0) { + sg_size_t chunk_size = std::min(this->buffer_size, remaining); #ifdef PAGE_CACHE_SIMULATION if (Simulation::isPageCachingEnabled()) { @@ -437,7 +438,7 @@ namespace wrench { } #endif - remaining -= this->buffer_size; + remaining -= chunk_size; if (req) { req->wait(); // WRENCH_INFO("Bytes sent over the network were received"); @@ -489,9 +490,9 @@ namespace wrench { } // Read the first chunk - double remaining = f->getSize(); - while (remaining > DBL_EPSILON) { - double to_read = std::min(remaining, this->buffer_size); + sg_size_t remaining = f->getSize(); + while (remaining > 0) { + sg_size_t to_read = std::min(remaining, this->buffer_size); int unique_disk_sequence_number_read = this->simulation_->getOutput().addTimestampDiskReadStart(Simulation::getCurrentSimulatedDate(), hostname, src_opened_file->get_path(), to_read); src_opened_file->read(to_read); this->simulation_->getOutput().addTimestampDiskReadCompletion(Simulation::getCurrentSimulatedDate(), hostname, src_opened_file->get_path(), to_read, unique_disk_sequence_number_read); @@ -522,8 +523,8 @@ namespace wrench { f->getID().c_str(), src_loc->toString().c_str()); // Check that the buffer size is compatible - if (((this->buffer_size < DBL_EPSILON) && (src_loc->getStorageService()->getBufferSize() > DBL_EPSILON)) or - ((this->buffer_size > DBL_EPSILON) && (src_loc->getStorageService()->getBufferSize() < DBL_EPSILON))) { + if (((this->buffer_size == 0) && (src_loc->getStorageService()->getBufferSize() > 0)) or + ((this->buffer_size > 0) && (src_loc->getStorageService()->getBufferSize() == 0))) { throw std::invalid_argument("FileTransferThread::downloadFileFromStorageService(): " "Incompatible buffer size specs (both must be zero, or both must be non-zero"); } diff --git a/test/simulated_failures/link_failures/StorageServiceLinkFailuresTest.cpp b/test/simulated_failures/link_failures/StorageServiceLinkFailuresTest.cpp index dddbe62728..21eff16605 100644 --- a/test/simulated_failures/link_failures/StorageServiceLinkFailuresTest.cpp +++ b/test/simulated_failures/link_failures/StorageServiceLinkFailuresTest.cpp @@ -74,8 +74,8 @@ class StorageServiceLinkFailuresTest : public ::testing::Test { } xml += - " " - " " + " " + " " " " " test = test; this->rng.seed(666); } @@ -259,7 +259,6 @@ class StorageServiceLinkFailuresTestWMS : public wrench::ExecutionController { for (unsigned long i = 0; i < NUM_TRIALS; i++) { // Do a random synchronous file copy - std::cerr << "DO A RANDOM SYNCHRONOUS FILE COPY\n"; try { this->doRandomSynchronousFileCopy(); } catch (wrench::ExecutionException &e) { @@ -271,7 +270,6 @@ class StorageServiceLinkFailuresTestWMS : public wrench::ExecutionController { wrench::Simulation::sleep(5); // Do a random asynchronous copy - std::cerr << "DO A RANDOM ASYNCHRONOUS FILE COPY\n"; try { this->doRandomAsynchronousFileCopy(); } catch (wrench::ExecutionException &e) { @@ -282,7 +280,6 @@ class StorageServiceLinkFailuresTestWMS : public wrench::ExecutionController { wrench::Simulation::sleep(5); - std::cerr << "DO A RANDOM FILE DELETE\n"; // Do a random delete try { this->doRandomFileDelete(); @@ -295,7 +292,6 @@ class StorageServiceLinkFailuresTestWMS : public wrench::ExecutionController { wrench::Simulation::sleep(5); // Do a random file read - std::cerr << "DO A RANDOM FILE READ\n"; try { this->doRandomFileRead(); } catch (wrench::ExecutionException &e) { @@ -307,7 +303,6 @@ class StorageServiceLinkFailuresTestWMS : public wrench::ExecutionController { wrench::Simulation::sleep(5); // Do a random file write - std::cerr << "DO A RANDOM FILE WRITE\n"; try { this->doRandomFileWrite(); } catch (wrench::ExecutionException &e) { diff --git a/test/simulation/simulation_output/SimulationTimestampFileCopyTest.cpp b/test/simulation/simulation_output/SimulationTimestampFileCopyTest.cpp index 2627c3eac6..c7052d0645 100755 --- a/test/simulation/simulation_output/SimulationTimestampFileCopyTest.cpp +++ b/test/simulation/simulation_output/SimulationTimestampFileCopyTest.cpp @@ -72,7 +72,7 @@ class SimulationTimestampFileCopyTest : public ::testing::Test { file_3 = wrench::Simulation::addFile("file_3", 100); xl_file = wrench::Simulation::addFile("xl_file", 1000000000ULL); - too_large_file = wrench::Simulation::addFile("too_large_file", 10000000000000000ULL); + too_large_file = wrench::Simulation::addFile("too_large_file", 1000000000000000ULL); } std::string platform_file_path = UNIQUE_TMP_PATH_PREFIX + "platform.xml"; diff --git a/test/workflow/WorkflowTaskTest.cpp b/test/workflow/WorkflowTaskTest.cpp index 6c41c50577..97ab56b945 100644 --- a/test/workflow/WorkflowTaskTest.cpp +++ b/test/workflow/WorkflowTaskTest.cpp @@ -29,7 +29,7 @@ class WorkflowTaskTest : public ::testing::Test { std::shared_ptr t1, t2, t4, t5, t6; std::shared_ptr large_input_file, small_input_file, t4_output_file; - void do_WorkflowTaskExecutionHistory_test(double buffer_size); + void do_WorkflowTaskExecutionHistory_test(sg_size_t buffer_size); protected: ~WorkflowTaskTest() { @@ -348,12 +348,12 @@ TEST_F(WorkflowTaskTest, WorkflowTaskExecutionHistoryTest) { DO_TEST_WITH_FORK_ONE_ARG(do_WorkflowTaskExecutionHistory_test, 0); } -void WorkflowTaskTest::do_WorkflowTaskExecutionHistory_test(double buffer_size) { +void WorkflowTaskTest::do_WorkflowTaskExecutionHistory_test(sg_size_t buffer_size) { auto simulation = wrench::Simulation::createSimulation(); int argc = 1; char **argv = (char **) calloc(argc, sizeof(char *)); argv[0] = strdup("unit_test"); -// argv[1] = strdup("--wrench-full-log"); +// argv[1] = strdup("--wrench-full-log"); ASSERT_NO_THROW(simulation->init(&argc, argv));