Skip to content

Commit

Permalink
Improve parsing performance (#234)
Browse files Browse the repository at this point in the history
* Convert some linear operations to constant time

* Temporary test command for performance testing

* Improve SplitArguments docs

* Re-enable split arguments unit test

* Restore repeat example
  • Loading branch information
natecook1000 authored Sep 1, 2020
1 parent 365ca6a commit db24cb1
Show file tree
Hide file tree
Showing 7 changed files with 247 additions and 159 deletions.
2 changes: 1 addition & 1 deletion Sources/ArgumentParser/Parsing/ArgumentDecoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ final class ArgumentDecoder: Decoder {
self.usedOrigins = InputOrigin()

// Mark the terminator position(s) as used:
values.elements.filter { $0.key == .terminator }.forEach {
values.elements.values.filter { $0.key == .terminator }.forEach {
usedOrigins.formUnion($0.inputOrigin)
}
}
Expand Down
14 changes: 8 additions & 6 deletions Sources/ArgumentParser/Parsing/ArgumentSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -316,18 +316,20 @@ extension ArgumentSet {
}
}

var result = ParsedValues(elements: [], originalInput: all.originalInput)
var usedOrigins = InputOrigin()
var result = ParsedValues(elements: [:], originalInput: all.originalInput)
var allUsedOrigins = InputOrigin()

try setInitialValues(into: &result)

// Loop over all arguments:
while let (origin, next) = inputArguments.popNext() {
var usedOrigins = InputOrigin()
defer {
inputArguments.removeAll(in: usedOrigins)
allUsedOrigins.formUnion(usedOrigins)
}

switch next {
switch next.value {
case .value:
// We'll parse positional values later.
break
Expand Down Expand Up @@ -359,9 +361,9 @@ extension ArgumentSet {
// We have parsed all non-positional values at this point.
// Next: parse / consume the positional values.
var unusedArguments = all
unusedArguments.removeAll(in: usedOrigins)
unusedArguments.removeAll(in: allUsedOrigins)
try parsePositionalValues(from: unusedArguments, into: &result)

return result
}
}
Expand Down Expand Up @@ -407,7 +409,7 @@ extension ArgumentSet {
var argumentStack = unusedInput.elements.filter {
$0.index.subIndex == .complete
}.map {
(InputOrigin.Element.argumentIndex($0.index), $0.element)
(InputOrigin.Element.argumentIndex($0.index), $0)
}[...]

guard !argumentStack.isEmpty else { return }
Expand Down
14 changes: 7 additions & 7 deletions Sources/ArgumentParser/Parsing/CommandParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ extension CommandParser {
// We should have used up all arguments at this point:
guard !split.containsNonTerminatorArguments else {
// Check if one of the arguments is an unknown option
for (index, element) in split.elements {
if case .option(let argument) = element {
throw ParserError.unknownOption(InputOrigin.Element.argumentIndex(index), argument.name)
for element in split.elements {
if case .option(let argument) = element.value {
throw ParserError.unknownOption(InputOrigin.Element.argumentIndex(element.index), argument.name)
}
}

Expand Down Expand Up @@ -296,12 +296,12 @@ extension CommandParser {

// Generate the argument set and parse the argument to find in the set
let argset = ArgumentSet(current.element)
let (_, parsedArgument) = try! parseIndividualArg(argToMatch, at: 0).first!
let parsedArgument = try! parseIndividualArg(argToMatch, at: 0).first!

// Look up the specified argument and retrieve its custom completion function
let completionFunction: ([String]) -> [String]

switch parsedArgument {
switch parsedArgument.value {
case .option(let parsed):
guard let matchedArgument = argset.first(matching: parsed),
case .custom(let f) = matchedArgument.completion.kind
Expand Down Expand Up @@ -362,7 +362,7 @@ extension CommandParser {
extension SplitArguments {
func contains(_ needle: Name) -> Bool {
self.elements.contains {
switch $0.element {
switch $0.value {
case .option(.name(let name)),
.option(.nameWithValue(let name, _)):
return name == needle
Expand All @@ -374,7 +374,7 @@ extension SplitArguments {

func contains(anyOf names: [Name]) -> Bool {
self.elements.contains {
switch $0.element {
switch $0.value {
case .option(.name(let name)),
.option(.nameWithValue(let name, _)):
return names.contains(name)
Expand Down
14 changes: 7 additions & 7 deletions Sources/ArgumentParser/Parsing/ParsedValues.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct ParsedValues {
}

/// These are the parsed key-value pairs.
var elements: [Element] = []
var elements: [InputKey: Element] = [:]

/// This is the *original* array of arguments that this was parsed from.
///
Expand All @@ -50,20 +50,20 @@ extension ParsedValues {
}

mutating func set(_ element: Element) {
if let index = elements.firstIndex(where: { $0.key == element.key }) {
if let e = elements[element.key] {
// Merge the source values. We need to keep track
// of any previous source indexes we have used for
// this key.
var e = element
e.inputOrigin.formUnion(elements[index].inputOrigin)
elements[index] = e
var element = element
element.inputOrigin.formUnion(e.inputOrigin)
elements[element.key] = element
} else {
elements.append(element)
elements[element.key] = element
}
}

func element(forKey key: InputKey) -> Element? {
return elements.first(where: { $0.key == key })
elements[key]
}

mutating func update<A>(forKey key: InputKey, inputOrigin: InputOrigin, initial: A, closure: (inout A) -> Void) {
Expand Down
Loading

0 comments on commit db24cb1

Please sign in to comment.