address comments about only finding routeable processes

phusion · Nov 11, 2024 · 8a5ebc3 · 8a5ebc3
1 parent d613e48
commit 8a5ebc3
Show file tree

Hide file tree

Showing 2 changed files with 2 additions and 97 deletions.
diff --git a/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp b/src/agent/Core/ApplicationPool/Group/ProcessListManagement.cpp
@@ -75,21 +75,12 @@ Group::findProcessWithStickySessionId(unsigned int id) const {
 Process *
 Group::findBestProcessPreferringStickySessionId(unsigned int id) const {
 	Process *bestProcess = nullptr;
-	Process *fallbackProcess = nullptr;
 	ProcessList::const_iterator it;
 	ProcessList::const_iterator end = enabledProcesses.end();
 	for (it = enabledProcesses.begin(); it != end; it++) {
 		Process *process = it->get();
 		if (process->getStickySessionId() == id) {
 			return process;
-		} else if (process->isTotallyBusy() && bestProcess == nullptr) {
-			if (fallbackProcess == nullptr ||
-				process->generation > fallbackProcess->generation ||
-				(process->generation == fallbackProcess->generation && process->spawnStartTime < fallbackProcess->spawnStartTime) ||
-				(process->generation == fallbackProcess->generation && process->spawnStartTime == fallbackProcess->spawnStartTime && process->busyness() < fallbackProcess->busyness())
-			) {
-				fallbackProcess = process;
-			}
 		} else if (!process->isTotallyBusy() && (bestProcess == nullptr ||
 				   process->generation > bestProcess->generation ||
 				   (process->generation == bestProcess->generation && process->spawnStartTime < bestProcess->spawnStartTime) ||
@@ -98,9 +89,6 @@ Group::findBestProcessPreferringStickySessionId(unsigned int id) const {
 			bestProcess = process;
 		}
 	}
-	if (bestProcess == nullptr) {
-		return fallbackProcess;
-	}
 	return bestProcess;
 }
 
@@ -120,88 +108,19 @@ Group::findBestProcess(const ProcessList &processes) const {
 	}
 
 	Process *bestProcess = nullptr;
-	Process *fallbackProcess = nullptr;
 	ProcessList::const_iterator it;
 	ProcessList::const_iterator end = processes.end();
 	for (it = processes.begin(); it != end; it++) {
 		Process *process = (*it).get();
 
-	 if (process->isTotallyBusy() && bestProcess == nullptr) {
-			if (fallbackProcess == nullptr ||
-				process->generation > fallbackProcess->generation ||
-				(process->generation == fallbackProcess->generation && process->spawnStartTime < fallbackProcess->spawnStartTime) ||
-				(process->generation == fallbackProcess->generation && process->spawnStartTime == fallbackProcess->spawnStartTime && process->busyness() < fallbackProcess->busyness())
-			) {
-				fallbackProcess = process;
-			}
-	 } else if (!process->isTotallyBusy() && (bestProcess == nullptr ||
+	 if (!process->isTotallyBusy() && (bestProcess == nullptr ||
 			process->generation > bestProcess->generation ||
 			(process->generation == bestProcess->generation && process->spawnStartTime < bestProcess->spawnStartTime) ||
 			(process->generation == bestProcess->generation && process->spawnStartTime == bestProcess->spawnStartTime && process->busyness() < bestProcess->busyness())
 		)) {
 			bestProcess = process;
 		}
 	}
-	if (bestProcess == nullptr) {
-		return fallbackProcess;
-	}
-	return bestProcess;
-}
-
-/**
- * Cache-optimized version of `findBestProcess()` for the common case.
- * See `findBestProcess()` for the general contract.
- */
-Process *
-Group::findBestEnabledProcess() const {
-	if (enabledProcesses.empty()) {
-		return nullptr;
-	}
-
-	Process *bestProcess = nullptr;
-	unsigned long long bestProcessStartTime = 0;
-	unsigned int bestProcessGen = 0;
-	int bestProcessBusyness = 0;
-
-	Process *fallbackProcess = nullptr;
-	unsigned long long fallbackProcessStartTime = 0;
-	unsigned int fallbackProcessGen = 0;
-	int fallbackProcessBusyness = 0;
-
-	unsigned int size = enabledProcessBusynessLevels.size();
-	const int *enabledProcessBusynessLevels = &this->enabledProcessBusynessLevels[0];
-
-	for (unsigned int i = 0; i < size; i++) {
-		Process *process = enabledProcesses.at(i).get();
-		unsigned int gen = process->generation;
-		unsigned long long startTime = process->spawnStartTime;
-		int busyness = enabledProcessBusynessLevels[i];
-		bool totallyBusy = process->isTotallyBusy();
-		if (totallyBusy && bestProcess == nullptr) {
-			if (fallbackProcess == nullptr ||
- 				gen > fallbackProcess->generation ||
-				(gen == fallbackProcessGen && startTime <  fallbackProcessStartTime) ||
-				(gen == fallbackProcessGen && startTime == fallbackProcessStartTime && busyness < fallbackProcessBusyness)
-			) {
-				fallbackProcess = process;
-				fallbackProcessGen = gen;
-				fallbackProcessBusyness = busyness;
-				fallbackProcessStartTime = startTime;
-			}
-		} else if (!totallyBusy && (bestProcess == nullptr ||
- 			gen > bestProcess->generation ||
-			(gen == bestProcessGen && startTime <  bestProcessStartTime) ||
-			(gen == bestProcessGen && startTime == bestProcessStartTime && busyness < bestProcessBusyness)
-		)) {
-			bestProcess = process;
-			bestProcessGen = gen;
-			bestProcessBusyness = busyness;
-			bestProcessStartTime = startTime;
-		}
-	}
-	if (bestProcess == nullptr) {
-		return fallbackProcess;
-	}
 	return bestProcess;
 }
 

diff --git a/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp b/src/agent/Core/ApplicationPool/Group/SessionManagement.cpp
@@ -65,15 +65,8 @@ Group::RouteResult
 Group::route(const Options &options) const {
 	if (OXT_LIKELY(enabledCount > 0)) {
 		if (options.stickySessionId == 0) {
-			Process *process = findBestEnabledProcess();
+			Process *process = findBestProcess(enabledProcesses);
 			if (process != nullptr) {
-				// TODO: remove this part when findBestEnabledProcess() has been
-				// modified to adhere to the new contract with postcondition
-				// `result != nullptr || result.canBeRoutedTo()`
-				if (!process->canBeRoutedTo()) {
-					return RouteResult(nullptr, false);
-				}
-
 				assert(process->canBeRoutedTo());
 				return RouteResult(process);
 			} else {
@@ -95,13 +88,6 @@ Group::route(const Options &options) const {
 	} else {
 		Process *process = findBestProcess(disablingProcesses);
 		if (process != nullptr) {
-			// TODO: remove this part when findBestProcess() has been
-			// modified to adhere to the new contract with postcondition
-			// `result != nullptr || result.canBeRoutedTo()`
-			if (!process->canBeRoutedTo()) {
-				return RouteResult(nullptr, false);
-			}
-
 			assert(process->canBeRoutedTo());
 			return RouteResult(process);
 		} else {