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

Optimize iterators a bit #1363

Merged
merged 1 commit into from
Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 51 additions & 0 deletions Jint.Benchmark/ForBenchmark.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using BenchmarkDotNet.Attributes;

namespace Jint.Benchmark;

[MemoryDiagnoser]
public class ForBencmark
{
private const string script = @"
var stringArray = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11', '12', '13', '14', '15', '16', '17', '18', '19', '20', '21', '22', '23', '24', '25', '26', '27', '28', '29', '30', '31', '32', '33', '34', '35', '36', '37', '38', '39', '40', '41', '42', '43', '44', '45', '46', '47', '48', '49', '50', '51', '52', '53', '54', '55', '56', '57', '58', '59', '60', '61', '62', '63', '64', '65', '66', '67', '68', '69', '70', '71', '72', '73', '74', '75', '76', '77', '78', '79', '80', '81', '82', '83', '84', '85', '86', '87', '88', '89', '90', '91', '92', '93', '94', '95', '96', '97', '98', '99', '100', '101', '102', '103', '104', '105', '106', '107', '108', '109', '110', '111', '112', '113', '114', '115', '116', '117', '118', '119', '120', '121', '122', '123', '124', '125', '126', '127', '128', '129', '130', '131', '132', '133', '134', '135', '136', '137', '138', '139', '140', '141', '142', '143', '144', '145', '146', '147', '148', '149', '150', '151', '152', '153', '154', '155', '156', '157', '158', '159', '160', '161', '162', '163', '164', '165', '166', '167', '168', '169', '170', '171', '172', '173', '174', '175', '176', '177', '178', '179', '180', '181', '182', '183', '184', '185', '186', '187', '188', '189', '190', '191', '192', '193', '194', '195', '196', '197', '198', '199', '200', '201', '202', '203', '204', '205', '206', '207', '208', '209', '210', '211', '212', '213', '214', '215', '216', '217', '218', '219', '220', '221', '222', '223', '224', '225', '226', '227', '228', '229', '230', '231', '232', '233', '234', '235', '236', '237', '238', '239', '240', '241', '242', '243', '244', '245', '246', '247', '248', '249', '250', '251', '252', '253', '254', '255', '256', '257', '258', '259', '260', '261', '262', '263', '264', '265', '266', '267', '268', '269', '270', '271', '272', '273', '274', '275', '276', '277', '278', '279', '280', '281', '282', '283', '284', '285', '286', '287', '288', '289', '290', '291', '292', '293', '294', '295', '296', '297', '298', '299', '300', '301', '302', '303', '304', '305', '306', '307', '308', '309', '310', '311', '312', '313', '314', '315', '316', '317', '318', '319', '320', '321', '322', '323', '324', '325', '326', '327', '328', '329', '330', '331', '332', '333', '334', '335', '336', '337', '338', '339', '340', '341', '342', '343', '344', '345', '346', '347', '348', '349', '350', '351', '352', '353', '354', '355', '356', '357', '358', '359', '360', '361', '362', '363', '364', '365', '366', '367', '368', '369', '370', '371', '372', '373', '374', '375', '376', '377', '378', '379', '380', '381', '382', '383', '384', '385', '386', '387', '388', '389', '390', '391', '392', '393', '394', '395', '396', '397', '398', '399', '400', '401', '402', '403', '404', '405', '406', '407', '408', '409', '410', '411', '412', '413', '414', '415', '416', '417', '418', '419', '420', '421', '422', '423', '424', '425', '426', '427', '428', '429', '430', '431', '432', '433', '434', '435', '436', '437', '438', '439', '440', '441', '442', '443', '444', '445', '446', '447', '448', '449', '450', '451', '452', '453', '454', '455', '456', '457', '458', '459', '460', '461', '462', '463', '464', '465', '466', '467', '468', '469', '470', '471', '472', '473', '474', '475', '476', '477', '478', '479', '480', '481', '482', '483', '484', '485', '486', '487', '488', '489', '490', '491', '492', '493', '494', '495', '496', '497', '498', '499', '500', '501', '502', '503', '504', '505', '506', '507', '508', '509', '510', '511', '512', '513', '514', '515', '516', '517', '518', '519', '520', '521', '522', '523', '524', '525', '526', '527', '528', '529', '530', '531', '532', '533', '534', '535', '536', '537', '538', '539', '540', '541', '542', '543', '544', '545', '546', '547', '548', '549', '550', '551', '552', '553', '554', '555', '556', '557', '558', '559', '560', '561', '562', '563', '564', '565', '566', '567', '568', '569', '570', '571', '572', '573', '574', '575', '576', '577', '578', '579', '580', '581', '582', '583', '584', '585', '586', '587', '588', '589', '590', '591', '592', '593', '594', '595', '596', '597', '598', '599', '600', '601', '602', '603', '604', '605', '606', '607', '608', '609', '610', '611', '612', '613', '614', '615', '616', '617', '618', '619', '620', '621', '622', '623', '624', '625', '626', '627', '628', '629', '630', '631', '632', '633', '634', '635', '636', '637', '638', '639', '640', '641', '642', '643', '644', '645', '646', '647', '648', '649', '650', '651', '652', '653', '654', '655', '656', '657', '658', '659', '660', '661', '662', '663', '664', '665', '666', '667', '668', '669', '670', '671', '672', '673', '674', '675', '676', '677', '678', '679', '680', '681', '682', '683', '684', '685', '686', '687', '688', '689', '690', '691', '692', '693', '694', '695', '696', '697', '698', '699', '700', '701', '702', '703', '704', '705', '706', '707', '708', '709', '710', '711', '712', '713', '714', '715', '716', '717', '718', '719', '720', '721', '722', '723', '724', '725', '726', '727', '728', '729', '730', '731', '732', '733', '734', '735', '736', '737', '738', '739', '740', '741', '742', '743', '744', '745', '746', '747', '748', '749', '750', '751', '752', '753', '754', '755', '756', '757', '758', '759', '760', '761', '762', '763', '764', '765', '766', '767', '768', '769', '770', '771', '772', '773', '774', '775', '776', '777', '778', '779', '780', '781', '782', '783', '784', '785', '786', '787', '788', '789', '790', '791', '792', '793', '794', '795', '796', '797', '798', '799', '800', '801', '802', '803', '804', '805', '806', '807', '808', '809', '810', '811', '812', '813', '814', '815', '816', '817', '818', '819', '820', '821', '822', '823', '824', '825', '826', '827', '828', '829', '830', '831', '832', '833', '834', '835', '836', '837', '838', '839', '840', '841', '842', '843', '844', '845', '846', '847', '848', '849', '850', '851', '852', '853', '854', '855', '856', '857', '858', '859', '860', '861', '862', '863', '864', '865', '866', '867', '868', '869', '870', '871', '872', '873', '874', '875', '876', '877', '878', '879', '880', '881', '882', '883', '884', '885', '886', '887', '888', '889', '890', '891', '892', '893', '894', '895', '896', '897', '898', '899', '900', '901', '902', '903', '904', '905', '906', '907', '908', '909', '910', '911', '912', '913', '914', '915', '916', '917', '918', '919', '920', '921', '922', '923', '924', '925', '926', '927', '928', '929', '930', '931', '932', '933', '934', '935', '936', '937', '938', '939', '940', '941', '942', '943', '944', '945', '946', '947', '948', '949', '950', '951', '952', '953', '954', '955', '956', '957', '958', '959', '960', '961', '962', '963', '964', '965', '966', '967', '968', '969', '970', '971', '972', '973', '974', '975', '976', '977', '978', '979', '980', '981', '982', '983', '984', '985', '986', '987', '988', '989', '990', '991', '992', '993', '994', '995', '996', '997', '998', '999', '1000'];
";

private Engine engine;


[GlobalSetup]
public void Setup()
{
engine = new Engine();
engine.Execute(script);
}

[Benchmark]
public void ForOfVar()
{
engine.Execute("for (var value of stringArray) { }");
}

[Benchmark]
public void ForOfLet()
{
engine.Execute("for (let value of stringArray) { }");
}

[Benchmark]
public void ForOfConst()
{
engine.Execute("for (const value of stringArray) { }");
}

[Benchmark]
public void ForVar()
{
engine.Execute("for (var i = 0; i < stringArray.length; ++i) { }");
}

[Benchmark]
public void ForLet()
{
engine.Execute("for (let i = 0; i < stringArray.length; ++i) { }");
}
}
4 changes: 2 additions & 2 deletions Jint.Tests/Runtime/EngineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ public void ShouldInvokeAFunctionValueThatBelongsToAnObject()
");

var obj = _engine.GetValue("obj").AsObject();
var getFoo = obj.Get("getFoo", obj);
var getFoo = obj.Get("getFoo");

Assert.Equal("foo is 5, bar is 7", _engine.Invoke(getFoo, obj, new object[] { 7 }).AsString());
}
Expand All @@ -881,7 +881,7 @@ public void ShouldNotInvokeNonFunctionValueThatBelongsToAnObject()
");

var obj = _engine.GetValue("obj").AsObject();
var foo = obj.Get("foo", obj);
var foo = obj.Get("foo");

Assert.Throws<JavaScriptException>(() => _engine.Invoke(foo, obj, new object[] { }));
}
Expand Down
2 changes: 1 addition & 1 deletion Jint.Tests/Runtime/JsValueConversionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void ShouldBeAnArray()
[Fact]
public void ShouldBeABoolean()
{
var value = new JsBoolean(true);
var value = JsBoolean.True;
Assert.Equal(true, value.IsBoolean());
Assert.Equal(false, value.IsArray());
Assert.Equal(false, value.IsDate());
Expand Down
9 changes: 9 additions & 0 deletions Jint/Native/Array/ArrayInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,15 @@ public sealed override JsValue Get(JsValue property, JsValue receiver)
return value;
}

if (property == CommonProperties.Length)
{
var length = _length?._value;
if (length is not null)
{
return length;
}
}

return base.Get(property, receiver);
}

Expand Down
14 changes: 7 additions & 7 deletions Jint/Native/Array/ArrayIteratorPrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,25 +85,25 @@ public override bool TryIteratorStep(out ObjectInstance nextItem)
{
nextItem = _kind switch
{
ArrayIteratorType.Key => new ValueIteratorPosition(_engine, _position),
ArrayIteratorType.Value => new ValueIteratorPosition(_engine, _typedArray[(int) _position]),
_ => new KeyValueIteratorPosition(_engine, _position, _typedArray[(int) _position])
ArrayIteratorType.Key => IteratorResult.CreateValueIteratorPosition(_engine, JsNumber.Create(_position)),
ArrayIteratorType.Value => IteratorResult.CreateValueIteratorPosition(_engine, _typedArray[(int) _position]),
_ => IteratorResult.CreateKeyValueIteratorPosition(_engine, JsNumber.Create(_position), _typedArray[(int) _position])
};
}
else
{
_operations!.TryGetValue(_position, out var value);
if (_kind == ArrayIteratorType.Key)
{
nextItem = new ValueIteratorPosition(_engine, _position);
nextItem = IteratorResult.CreateValueIteratorPosition(_engine, JsNumber.Create(_position));
}
else if (_kind == ArrayIteratorType.Value)
{
nextItem = new ValueIteratorPosition(_engine, value);
nextItem = IteratorResult.CreateValueIteratorPosition(_engine, value);
}
else
{
nextItem = new KeyValueIteratorPosition(_engine, _position, value);
nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine, JsNumber.Create(_position), value);
}
}

Expand All @@ -112,7 +112,7 @@ public override bool TryIteratorStep(out ObjectInstance nextItem)
}

_closed = true;
nextItem = KeyValueIteratorPosition.Done(_engine);
nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine);
return false;
}
}
Expand Down
6 changes: 3 additions & 3 deletions Jint/Native/Array/ArrayOperations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public ObjectInstanceOperations(ObjectInstance target) : base(target)

private double GetIntegerLength()
{
var descValue = _target.Get(CommonProperties.Length, _target);
var descValue = _target.Get(CommonProperties.Length);
if (!ReferenceEquals(descValue, null))
{
return TypeConverter.ToInteger(descValue);
Expand Down Expand Up @@ -185,7 +185,7 @@ public override void EnsureCapacity(ulong capacity)
}

public override JsValue Get(ulong index)
=> _target.Get(JsString.Create(index), _target);
=> _target.Get(JsString.Create(index));

public override bool TryGetValue(ulong index, out JsValue value)
{
Expand Down Expand Up @@ -297,7 +297,7 @@ public override uint GetLength()
return _target.Length;
}

var descValue = _target.Get(CommonProperties.Length, _target);
var descValue = _target.Get(CommonProperties.Length);
if (!ReferenceEquals(descValue, null))
{
return (uint) TypeConverter.ToInteger(descValue);
Expand Down
4 changes: 2 additions & 2 deletions Jint/Native/Array/ArrayPrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,7 @@ private JsValue ToLocaleString(JsValue thisObj, JsValue[] arguments)
else
{
var elementObj = TypeConverter.ToObject(_realm, firstElement);
var func = elementObj.Get("toLocaleString", elementObj) as ICallable;
var func = elementObj.Get("toLocaleString") as ICallable;
if (func is null)
{
ExceptionHelper.ThrowTypeError(_realm);
Expand All @@ -1292,7 +1292,7 @@ private JsValue ToLocaleString(JsValue thisObj, JsValue[] arguments)
else
{
var elementObj = TypeConverter.ToObject(_realm, nextElement);
var func = elementObj.Get("toLocaleString", elementObj) as ICallable;
var func = elementObj.Get("toLocaleString") as ICallable;
if (func is null)
{
ExceptionHelper.ThrowTypeError(_realm);
Expand Down
2 changes: 1 addition & 1 deletion Jint/Native/Date/DatePrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ private JsValue ToJSON(JsValue thisObj, JsValue[] arguments)
return Null;
}

var toIso = o.Get("toISOString", o);
var toIso = o.Get("toISOString");
var callable = toIso as ICallable;
if (callable is null)
{
Expand Down
2 changes: 1 addition & 1 deletion Jint/Native/Function/FunctionInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ internal void SetFunctionName(JsValue name, string? prefix = null, bool force =
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal ObjectInstance GetPrototypeFromConstructor(JsValue constructor, Func<Intrinsics, ObjectInstance> intrinsicDefaultProto)
{
if (constructor.Get(CommonProperties.Prototype, constructor) is not ObjectInstance proto)
if (constructor.Get(CommonProperties.Prototype) is not ObjectInstance proto)
{
var realm = GetFunctionRealm(constructor);
proto = intrinsicDefaultProto(realm.Intrinsics);
Expand Down
46 changes: 6 additions & 40 deletions Jint/Native/Iterator/IteratorInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using Jint.Native.Object;
using Jint.Native.RegExp;
using Jint.Runtime;
using Jint.Runtime.Descriptors;

namespace Jint.Native.Iterator
{
Expand Down Expand Up @@ -33,11 +32,11 @@ public virtual bool TryIteratorStep(out ObjectInstance nextItem)
{
if (_enumerable.MoveNext())
{
nextItem = new ValueIteratorPosition(_engine, _enumerable.Current);
nextItem = IteratorResult.CreateValueIteratorPosition(_engine, _enumerable.Current);
return true;
}

nextItem = ValueIteratorPosition.Done(_engine);
nextItem = IteratorResult.CreateValueIteratorPosition(_engine, done: JsBoolean.True);
return false;
}

Expand All @@ -50,40 +49,7 @@ public virtual void Close(CompletionType completion)
/// </summary>
private ObjectInstance CreateIterResultObject(JsValue value, bool done)
{
return new IteratorResult(_engine, value, done ? JsBoolean.True : JsBoolean.False);
}

internal sealed class KeyValueIteratorPosition : ObjectInstance
{
internal static ObjectInstance Done(Engine engine) => new KeyValueIteratorPosition(engine, null, null);

public KeyValueIteratorPosition(Engine engine, JsValue? key, JsValue? value) : base(engine)
{
var done = ReferenceEquals(null, key) && ReferenceEquals(null, value);
if (!done)
{
var arrayInstance = engine.Realm.Intrinsics.Array.ArrayCreate(2);
arrayInstance.SetIndexValue(0, key!, false);
arrayInstance.SetIndexValue(1, value!, false);
SetProperty("value", new PropertyDescriptor(arrayInstance, PropertyFlag.AllForbidden));
}
SetProperty("done", done ? PropertyDescriptor.AllForbiddenDescriptor.BooleanTrue : PropertyDescriptor.AllForbiddenDescriptor.BooleanFalse);
}
}

internal sealed class ValueIteratorPosition : ObjectInstance
{
internal static ObjectInstance Done(Engine engine, JsValue? value = null)
=> new ValueIteratorPosition(engine, value ?? Undefined, true);

public ValueIteratorPosition(Engine engine, JsValue value, bool? done = null) : base(engine)
{
if (value is not null)
{
SetProperty("value", new PropertyDescriptor(value, PropertyFlag.AllForbidden));
}
SetProperty("done", new PropertyDescriptor(done ?? value is null, PropertyFlag.AllForbidden));
}
return new IteratorResult(_engine, value, JsBoolean.Create(done));
}

internal sealed class ObjectIterator : IteratorInstance
Expand All @@ -94,7 +60,7 @@ internal sealed class ObjectIterator : IteratorInstance
public ObjectIterator(ObjectInstance target) : base(target.Engine)
{
_target = target;
if (target.Get(CommonProperties.Next, target) is not ICallable callable)
if (target.Get(CommonProperties.Next) is not ICallable callable)
{
ExceptionHelper.ThrowTypeError(target.Engine.Realm);
return;
Expand Down Expand Up @@ -173,11 +139,11 @@ public override bool TryIteratorStep(out ObjectInstance nextItem)
{
if (_iterator.MoveNext())
{
nextItem = new ValueIteratorPosition(_engine, (string) _iterator.Current);
nextItem = IteratorResult.CreateValueIteratorPosition(_engine, (string) _iterator.Current);
return true;
}

nextItem = KeyValueIteratorPosition.Done(_engine);
nextItem = IteratorResult.CreateKeyValueIteratorPosition(_engine);
return false;
}
}
Expand Down