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

smarter action shuffler #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ dependencies {
implementation("org.apache.commons:commons-math3:3.6.1")
implementation("com.atlassian.performance.tools:concurrency:[1.0.0,2.0.0)")
implementation("com.atlassian.performance:selenium-js:[1.0.0,2.0.0)")

runtimeOnly("org.jetbrains.kotlin:kotlin-reflect:${kotlinVersion}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Uh oh, reflection = danger zone. We're bypassing a lot of compilation-time guarantees.


listOf(
"api",
"core",
Expand Down
1 change: 1 addition & 0 deletions gradle/dependency-locks/default.lockfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ org.apache.logging.log4j:log4j-slf4j-impl:2.10.0
org.checkerframework:checker-compat-qual:2.0.0
org.codehaus.mojo:animal-sniffer-annotations:1.14
org.glassfish:javax.json:1.1
org.jetbrains.kotlin:kotlin-reflect:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-common:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.2.70
Expand Down
1 change: 1 addition & 0 deletions gradle/dependency-locks/runtimeClasspath.lockfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ org.apache.logging.log4j:log4j-slf4j-impl:2.10.0
org.checkerframework:checker-compat-qual:2.0.0
org.codehaus.mojo:animal-sniffer-annotations:1.14
org.glassfish:javax.json:1.1
org.jetbrains.kotlin:kotlin-reflect:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-common:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.2.70
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# This is a Gradle generated file for dependency locking.
# Manual edits can break the build and are not advised.
# This file is expected to be part of source control.
org.jetbrains.kotlin:kotlin-reflect:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-common:1.2.70
org.jetbrains.kotlin:kotlin-stdlib:1.2.70
org.jetbrains:annotations:13.0
1 change: 1 addition & 0 deletions gradle/dependency-locks/testRuntimeClasspath.lockfile
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ org.checkerframework:checker-compat-qual:2.0.0
org.codehaus.mojo:animal-sniffer-annotations:1.14
org.glassfish:javax.json:1.1
org.hamcrest:hamcrest-core:1.3
org.jetbrains.kotlin:kotlin-reflect:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-common:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-jdk8:1.2.70
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# This is a Gradle generated file for dependency locking.
# Manual edits can break the build and are not advised.
# This file is expected to be part of source control.
org.jetbrains.kotlin:kotlin-reflect:1.2.70
org.jetbrains.kotlin:kotlin-stdlib-common:1.2.70
org.jetbrains.kotlin:kotlin-stdlib:1.2.70
org.jetbrains:annotations:13.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package com.atlassian.performance.tools.jiraactions.api.scenario

import com.atlassian.performance.tools.jiraactions.api.SeededRandom
import com.atlassian.performance.tools.jiraactions.api.WebJira
import com.atlassian.performance.tools.jiraactions.api.action.Action
import com.atlassian.performance.tools.jiraactions.api.action.BrowseProjectsAction
import com.atlassian.performance.tools.jiraactions.api.action.CreateIssueAction
import com.atlassian.performance.tools.jiraactions.api.action.SearchJqlAction
import com.atlassian.performance.tools.jiraactions.api.measure.ActionMeter
import com.atlassian.performance.tools.jiraactions.api.memories.IssueKeyMemory
import com.atlassian.performance.tools.jiraactions.api.memories.JqlMemory
import com.atlassian.performance.tools.jiraactions.api.page.IssuePage
import kotlin.reflect.KClass

internal class ActionShuffler {
private class FixedJqlMemory(val jql: String) : JqlMemory {
override fun observe(issuePage: IssuePage) {
throw UnsupportedOperationException()
}

override fun recall(): String? {
return jql
}

override fun remember(memories: Collection<String>) {
throw UnsupportedOperationException()
}
}

companion object {
fun createRandomisedScenario(seededRandom: SeededRandom, actionProportions: Map<Action, Int>,
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the problem we're trying to solve here?

Do skipped actions malform the ActionMetric.label proportions too much? Our initial workaround was to run the test for a longer time so they get averaged out.
If that is not effective enough, then we can avoid action skips by using a self-loading Memory, e.g. pass this to an action during Memory.recall:

fun recall(): String {
    if (issueKeys.isEmpty()) {
        SearchJqlAction(jira, throwawayActionMeter, IssueJqlMemory(), this).run()
    }
}

This way you can avoid skips with local logic rather than a global order-controlling scheme.

Copy link
Author

Choose a reason for hiding this comment

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

Yes. I want to preserve the requested proportions. During our runs, ~1500 view issue calls are skipped. This looks like a minimally invasive way of achieving this, without altering the way scenarios are created.
The problem with hiding the query inside a different action is that you lose control over proportions - if there are permission issues, the JQL action can be executed as often as view issue, which we don't want. If we stick to sorting, everything is transparent and there are no surprises hiding in the implicit initialisation.

Copy link
Author

Choose a reason for hiding this comment

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

The same problem affects ViewBoard, btw.

Copy link
Author

Choose a reason for hiding this comment

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

Let's try a different question: is there any benefit to keeping a fully-randomised shuffle in the scenario?

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔
I'll come back to this next week

issueKeyMemoriser: Action): List<Action> {
//createIssue needs a project - browserProject goes first
//viewIssue needs to have issues - createIssues goes second
return createRandomisedScenario(seededRandom, actionProportions, issueKeyMemoriser, BrowseProjectsAction::class, CreateIssueAction::class)
}

fun findIssueKeysWithJql(jira: WebJira, meter: ActionMeter, issueKeyMemory: IssueKeyMemory): SearchJqlAction {
return SearchJqlAction(
jira = jira,
meter = meter,
jqlMemory = FixedJqlMemory("project is not EMPTY"),
issueKeyMemory = issueKeyMemory
)
}

private fun createRandomisedScenario(seededRandom: SeededRandom, actionProportions: Map<Action, Int>,
issueKeyDiscoverer: Action, vararg actions: KClass<out Action>): List<Action> {
val initialActions = findActions(actionProportions, *actions)

val scenario: MutableList<Action> = mutableListOf()

val actionProportionsToRandomise = deductActionCount(actionProportions, initialActions)
actionProportionsToRandomise.entries.forEach { scenario.addMultiple(element = it.key, repeats = it.value) }
scenario.shuffle(seededRandom.random)

//viewIssue needs to remember isssues - issueKeyDiscoverer goes after all actions
scenario.addAll(0, initialActions.plus(issueKeyDiscoverer))
return scenario
}

private fun findActions(actionProportions: Map<Action, Int>, vararg actions: KClass<out Action>): List<Action> {
return actions
.mapNotNull { findAction(actionProportions, it) }
}

private fun deductActionCount(actionProportions: Map<Action, Int>, actions: List<Action>): MutableMap<Action, Int> {
val modifiedActionProportions = actionProportions.toMutableMap()

actions.forEach {
val originalCount = actionProportions.getValue(it)
modifiedActionProportions[it] = originalCount-1
}
return modifiedActionProportions
}

private fun <T : Action> findAction(actionProportions: Map<Action, Int>, kClass: KClass<T>): T? {
@Suppress("UNCHECKED_CAST")
return actionProportions.keys.find { kClass.java.isInstance(it) } as T?
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ class JiraCoreScenario constructor() : Scenario {
val jqlMemory = AdaptiveJqlMemory(seededRandom)
val issueMemory = AdaptiveIssueMemory(issueKeyMemory, seededRandom)

val scenario: MutableList<Action> = mutableListOf()

val createIssue = CreateIssueAction(
jira = jira,
meter = meter,
Expand All @@ -41,6 +39,9 @@ class JiraCoreScenario constructor() : Scenario {
jqlMemory = jqlMemory,
issueKeyMemory = issueKeyMemory
)

val findInitialIssues = ActionShuffler.findIssueKeysWithJql(jira, meter, issueKeyMemory)

val viewIssue = ViewIssueAction(
jira = jira,
meter = meter,
Expand Down Expand Up @@ -75,7 +76,7 @@ class JiraCoreScenario constructor() : Scenario {

val actionProportions = mapOf(
createIssue to 5,
searchWithJql to 20,
searchWithJql to 19,
viewIssue to 55,
projectSummary to 5,
viewDashboard to 10,
Expand All @@ -84,9 +85,7 @@ class JiraCoreScenario constructor() : Scenario {
browseProjects to 5
)

actionProportions.entries.forEach { scenario.addMultiple(element = it.key, repeats = it.value) }
scenario.shuffle(seededRandom.random)
return scenario
return ActionShuffler.createRandomisedScenario(seededRandom, actionProportions, findInitialIssues)
}

private fun initializeIssueKeyMemory(seededRandom: SeededRandom) {
Expand Down