Skip to content

Improve performance in argument unpacking #1838

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Improve performance in argument unpacking #1838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a6cede
Cache parameters list
slozier Dec 11, 2024
365ad2d
Use Queue to speed up popping
slozier Dec 12, 2024
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
4 changes: 3 additions & 1 deletion Src/IronPython/Compiler/Ast/FunctionDefinition.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ internal override MSAst.Expression LocalContext {

public IList<Parameter> Parameters => _parameters;

internal override string[] ParameterNames => ArrayUtils.ConvertAll(_parameters, val => val.Name);
private string[] _parameterNames = null;

internal override string[] ParameterNames => _parameterNames ??= ArrayUtils.ConvertAll(_parameters, val => val.Name);

internal override int ArgCount {
get {
Expand Down
19 changes: 8 additions & 11 deletions Src/IronPython/Runtime/Binding/MetaPythonFunction.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ private bool TryFinishList(Expression[] exprArgs, List<Expression> paramsArgs) {
// make a single copy.
exprArgs[_func.Value.ExpandListPosition] = Ast.Call(
typeof(PythonOps).GetMethod(nameof(PythonOps.GetOrCopyParamsTuple)),
_codeContext ?? AstUtils.Constant(DefaultContext.Default),
GetFunctionParam(),
AstUtils.Convert(_userProvidedParams, typeof(object))
);
Expand Down Expand Up @@ -653,7 +654,7 @@ private void AddCheckForNoExtraParameters(Expression[] exprArgs) {
Ast.Call(
typeof(PythonOps).GetMethod(nameof(PythonOps.CheckParamsZero)),
AstUtils.Convert(GetFunctionParam(), typeof(PythonFunction)),
_params
_params // Queue<object>
)
);
} else if (_userProvidedParams != null) {
Expand Down Expand Up @@ -805,7 +806,7 @@ private Expression ExtractDefaultValue(string argName, int dfltIndex) {
AstUtils.Convert(GetFunctionParam(), typeof(PythonFunction)),
AstUtils.Constant(dfltIndex),
AstUtils.Constant(argName, typeof(string)),
VariableOrNull(_params, typeof(PythonList)),
VariableOrNull(_params, typeof(Queue<object>)),
VariableOrNull(_dict, typeof(PythonDictionary))
);
}
Expand Down Expand Up @@ -843,7 +844,7 @@ private Expression ExtractFromListOrDictionary(string name) {
AstUtils.Convert(GetFunctionParam(), typeof(PythonFunction)), // function
AstUtils.Constant(name, typeof(string)), // name
_paramsLen, // arg count
_params, // params list
_params, // Queue<object>
AstUtils.Convert(_dict, typeof(IDictionary)) // dictionary
);
}
Expand All @@ -860,17 +861,13 @@ private void EnsureParams() {
/// Helper function to extract the next argument from the params list.
/// </summary>
private Expression ExtractNextParamsArg() {
if (!_extractedParams) {
MakeParamsCopy(_userProvidedParams);

_extractedParams = true;
}
EnsureParams();

return Ast.Call(
typeof(PythonOps).GetMethod(nameof(PythonOps.ExtractParamsArgument)),
AstUtils.Convert(GetFunctionParam(), typeof(PythonFunction)), // function
AstUtils.Constant(Signature.ArgumentCount), // arg count
_params // list
_params // Queue<object>
);
}

Expand Down Expand Up @@ -945,7 +942,7 @@ private static Expression VariableOrNull(ParameterExpression var, Type type) {
private void MakeParamsCopy(Expression/*!*/ userList) {
Debug.Assert(_params == null);

_temps.Add(_params = Ast.Variable(typeof(PythonList), "$list"));
_temps.Add(_params = Ast.Variable(typeof(Queue<object>), "$list"));
_temps.Add(_paramsLen = Ast.Variable(typeof(int), "$paramsLen"));

EnsureInit();
Expand All @@ -965,7 +962,7 @@ private void MakeParamsCopy(Expression/*!*/ userList) {
_init.Add(
Ast.Assign(_paramsLen,
Ast.Add(
Ast.Call(_params, typeof(PythonList).GetMethod(nameof(PythonList.__len__))),
Ast.Property(_params, nameof(Queue<object>.Count)),
AstUtils.Constant(Signature.GetProvidedPositionalArgumentCount())
)
)
Expand Down
75 changes: 50 additions & 25 deletions Src/IronPython/Runtime/Operations/PythonOps.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ internal static bool TryInvokeLengthHint(CodeContext context, object? sequence,
return CallWithContext(context, func, args);
}

[Obsolete("Use ObjectOpertaions instead")]
[Obsolete("Use ObjectOperations instead")]
public static object? CallWithArgsTupleAndKeywordDictAndContext(CodeContext/*!*/ context, object func, object[] args, string[] names, object argsTuple, object kwDict) {
IDictionary? kws = kwDict as IDictionary;
if (kws == null && kwDict != null) throw PythonOps.TypeError("argument after ** must be a dictionary");
Expand Down Expand Up @@ -2634,11 +2634,26 @@ public static void VerifyUnduplicatedByName(PythonFunction function, string name
}


public static PythonList CopyAndVerifyParamsList(CodeContext context, PythonFunction function, object list) {
return new PythonList(context, list);
[EditorBrowsable(EditorBrowsableState.Never)]
public static Queue<object?> CopyAndVerifyParamsList(CodeContext context, PythonFunction function, object list) {
if (list is not IEnumerable<object?> e) {
if (!TryGetEnumerator(context, list, out IEnumerator? enumerator)) {
// TODO: CPython 3.5 uses "an iterable" in the error message instead of "a sequence"
throw TypeError($"{function.__name__}() argument after * must be a sequence, not {PythonOps.GetPythonTypeName(list)}");
}
e = IEnumerableFromEnumerator(enumerator);
}
return new Queue<object?>(e);

static IEnumerable<object?> IEnumerableFromEnumerator(IEnumerator ie) {
while (ie.MoveNext()) {
yield return ie.Current;
}
}
}

public static PythonTuple UserMappingToPythonTuple(CodeContext/*!*/ context, object list, string funcName) {
[EditorBrowsable(EditorBrowsableState.Never)]
public static PythonTuple UserMappingToPythonTuple(CodeContext/*!*/ context, object? list, string funcName) {
if (!TryGetEnumeratorObject(context, list, out object? enumerator)) {
// TODO: CPython 3.5 uses "an iterable" in the error message instead of "a sequence"
throw TypeError($"{funcName}() argument after * must be a sequence, not {PythonOps.GetPythonTypeName(list)}");
Expand All @@ -2647,44 +2662,52 @@ public static PythonTuple UserMappingToPythonTuple(CodeContext/*!*/ context, obj
return PythonTuple.Make(enumerator);
}

public static PythonTuple GetOrCopyParamsTuple(PythonFunction function, object input) {
if (input == null) {
throw PythonOps.TypeError("{0}() argument after * must be a sequence, not NoneType", function.__name__);
[EditorBrowsable(EditorBrowsableState.Never)]
public static PythonTuple GetOrCopyParamsTuple(CodeContext/*!*/ context, PythonFunction function, object? input) {
if (input is PythonTuple t && t.GetType() == typeof(PythonTuple)) {
return t;
}

return PythonTuple.Make(input);
return UserMappingToPythonTuple(context, input, function.__name__);
}

public static object? ExtractParamsArgument(PythonFunction function, int argCnt, PythonList list) {
if (list.__len__() != 0) {
return list.pop(0);
[EditorBrowsable(EditorBrowsableState.Never)]
public static object? ExtractParamsArgument(PythonFunction function, int argCnt, Queue<object?> list) {
if (list.Count != 0) {
return list.Dequeue();
}

throw function.BadArgumentError(argCnt);
}

public static void AddParamsArguments(PythonList list, params object[] args) {
for (int i = 0; i < args.Length; i++) {
list.insert(i, args[i]);
[EditorBrowsable(EditorBrowsableState.Never)]
public static void AddParamsArguments(Queue<object> list, params object[] args) {
var len = list.Count;
foreach (var arg in args) {
list.Enqueue(arg);
}
// put existing arguments at the end
for (int i = 0; i < len; i++) {
list.Enqueue(list.Dequeue());
}
}

/// <summary>
/// Extracts an argument from either the dictionary or params
/// </summary>
public static object? ExtractAnyArgument(PythonFunction function, string name, int argCnt, PythonList list, IDictionary dict) {
[EditorBrowsable(EditorBrowsableState.Never)]
public static object? ExtractAnyArgument(PythonFunction function, string name, int argCnt, Queue<object?> list, IDictionary dict) {
object? val;
if (dict.Contains(name)) {
if (list.__len__() != 0) {
if (list.Count != 0) {
throw MultipleKeywordArgumentError(function, name);
}
val = dict[name];
dict.Remove(name);
return val;
}

if (list.__len__() != 0) {
return list.pop(0);
if (list.Count != 0) {
return list.Dequeue();
}

if (function.ExpandDictPosition == -1 && dict.Count > 0) {
Expand Down Expand Up @@ -2726,9 +2749,10 @@ public static ArgumentTypeException SimpleTypeError(string message) {
return function.Defaults[index];
}

public static object? GetFunctionParameterValue(PythonFunction function, int index, string name, PythonList? extraArgs, PythonDictionary? dict) {
if (extraArgs != null && extraArgs.__len__() > 0) {
return extraArgs.pop(0);
[EditorBrowsable(EditorBrowsableState.Never)]
public static object? GetFunctionParameterValue(PythonFunction function, int index, string name, Queue<object?>? extraArgs, PythonDictionary? dict) {
if (extraArgs != null && extraArgs.Count > 0) {
return extraArgs.Dequeue();
}

if (dict != null && dict.TryRemoveValue(name, out object val)) {
Expand All @@ -2746,9 +2770,10 @@ public static ArgumentTypeException SimpleTypeError(string message) {
return function.__kwdefaults__?[name];
}

public static void CheckParamsZero(PythonFunction function, PythonList extraArgs) {
if (extraArgs.__len__() != 0) {
throw function.BadArgumentError(extraArgs.__len__() + function.NormalArgumentCount);
[EditorBrowsable(EditorBrowsableState.Never)]
public static void CheckParamsZero(PythonFunction function, Queue<object?> extraArgs) {
if (extraArgs.Count != 0) {
throw function.BadArgumentError(extraArgs.Count + function.NormalArgumentCount);
}
}

Expand Down
Loading