Refactor Loader.go, in preparation for adding new trace type.#740
Open
16fb wants to merge 7 commits intovhive-serverless:mainfrom
Open
Refactor Loader.go, in preparation for adding new trace type.#74016fb wants to merge 7 commits intovhive-serverless:mainfrom
16fb wants to merge 7 commits intovhive-serverless:mainfrom
Conversation
Deprecates GenerateSpecification(), ReadOrWriteFileSpecification() outputIATsToFile() Replaced with GenerateAzure2019Specification(), ReadOrWriteSpecificationToFile() Signed-off-by: Bryan Wong <wongwenpingbryan@gmail.com>
Requires each inputType's generator to return a standardised []*common.Function. Vswarn and Dirigent settings left to generator to handle. Signed-off-by: Bryan Wong <wongwenpingbryan@gmail.com>
Decomponsed RPS function generation (CreateRPSFunctions()) into function creation, and adding dirigent metadata to function. Extract Dirigent out of GenerateFunction(). Own dedicated section in runMode(). Signed-off-by: Bryan Wong wongwenpingbryan@gmail.com
Signed-off-by: Bryan Wong wongwenpingbryan@gmail.com
Update trace_driver test to work with updated function_specification generation. Removed unused, deprecated functions. Added back "FailureConfiguration:" property which was wrongly removed. Signed-off-by: Bryan Wong wongwenpingbryan@gmail.com
For functions: generator.GenerateAzure2019Specification(), generator.AppendDirigentMetadata(). Fixed function references, and to expect []*function to be updated when passed to func. Signed-off-by: Bryan Wong wongwenpingbryan@gmail.com
Remove shiftIAT and IATDistribution from loader configuration. Updated driver_tests to not reference the loader struct, reference the shiftIAT and IATDistribution directly. Filled up missing fields for loader creation in RunMode(). Signed-off-by: Bryan Wong wongwenpingbryan@gmail.com
Collaborator
Author
|
Previous 2 commits added your pointers to the PR |
Contributor
leokondrashov
left a comment
leokondrashov
left a comment
There was a problem hiding this comment.
Quite good now, please fix minor comments and ready to merge
| case "Azure2019", "vSwarm": | ||
| // Reads Dirigent trace meta-data, and places into *common.Function as property "dirigentMetadata" | ||
| // Or if Knative yaml is provided, converts it to dirigent configurations. | ||
| // Dirigent metadata parsing |
Contributor
There was a problem hiding this comment.
please go over the comments you added and prune them. Some of them are really not needed like this "Dirigent metadata parsing" are now redundant.
Comment on lines
+228
to
+230
| LoaderConfiguration: cfg, | ||
| FailureConfiguration: config.ReadFailureConfiguration(*failurePath), | ||
| DirigentConfiguration: dirigentConfig, |
Contributor
There was a problem hiding this comment.
did you run through gofmt? it seems weird that cfg got moved by one space but failure confif wasn't
| yamlPath := parseYAMLSpecification(cfg) | ||
| var functions []*common.Function | ||
| var traceParser trace.Parser | ||
| func runMode(cfg *config.LoaderConfiguration, readIATFromFile bool, writeIATsToFile bool) { |
Contributor
There was a problem hiding this comment.
I would call it just run at this point
| } | ||
|
|
||
| func CreateRPSFunctions(cfg *config.LoaderConfiguration, dcfg *config.DirigentConfig, warmFunction common.IATArray, warmFunctionCount []int, | ||
| // Generates []*common.Function based on intended warm/cold functionality, does not generate dirigent metadata. |
Contributor
There was a problem hiding this comment.
I would remove the part of the comment about what it doesn't do
| return result | ||
| } | ||
|
|
||
| // TODO consider pointer receiver for []*functions |
Contributor
There was a problem hiding this comment.
I think this one is redundant
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactored Loader.
Ensure more clear separation of function parsing/generation, dirigent reading, driver running.
Implementation Notes ⚒️
Added traceInputType variable classification
Combined runRPS() and runTrace() into singular runMode()
Driver should use []*common.function directly, extracted function generation out of driver.
Driver should not handle function read/write to file, extract out of driver.
Dirigent metadata adding extracted out of function generation, takes a dedicated section of runMode()
RPS function generation rewritten to extract dirigent metadata adding out.
External Dependencies 🍀
N/A
Breaking API Changes⚠️
trace_driver_test had to be updated to use new function specification generation method.
driver.GenerateSpecification() => GenerateAzure2019Specification()