Skip to content
2 changes: 2 additions & 0 deletions std/assembly.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,8 @@ declare class String {
trim(): string;
trimLeft(): string;
trimRight(): string;
repeat(count?: i32): string;
toString(): string;
}

/** Class for representing a runtime error. Base class of all errors. */
Expand Down
46 changes: 43 additions & 3 deletions std/assembly/string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ export class String {

@operator(">")
private static __gt(left: String, right: String): bool {
if (left === null || right === null) return false;
if (left === right || left === null || right === null) {
return false;
}

var leftLength = left.length;
var rightLength = right.length;
Expand All @@ -149,8 +151,9 @@ export class String {

@operator(">=")
private static __gte(left: String, right: String): bool {
if (left === right) return true;
if (left === null) return right === null;
else if (right === null) return false;
if (right === null) return false;

var leftLength = left.length;
var rightLength = right.length;
Expand All @@ -168,7 +171,9 @@ export class String {

@operator("<")
private static __lt(left: String, right: String): bool {
if (left === null || right === null) return false;
if (left === right || left === null || right === null) {
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

In stdlib, I'm trying to avoid such blocks. Doesn't look as nice, but the parser ultimately can skip producing one BlockStatement, slightly reducing memory footprint. Not sure if that's overoptimization already, but I had the feeling it might be okish to do :)

Copy link
Member Author

Choose a reason for hiding this comment

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

So I should refactor this to 3-timed if instead one block?

Copy link
Member

@dcodeIO dcodeIO Apr 7, 2018

Choose a reason for hiding this comment

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

Just if (left === right || left === null || right === null) return false;, so it results in the following AST:

IfStatement - Expression - ReturnStatement 

instead of

IfStatement - Expression - BlockStatement (with just one child) - ReturnStatement 
Copy link
Member Author

@MaxGraey MaxGraey Apr 7, 2018

Choose a reason for hiding this comment

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

I refactored this. But I can't see any changes even in untouched wast except assert's column/line numbers. Is it normal? It seems this overoptimization)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it does compile to the same thing, but the parser has to allocate one less AST node. My though was that, since stdlib is always included in compilation and fully parsed (unlike other compilers that might link it in), that it would accumulate.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, got it


var leftLength = left.length;
var rightLength = right.length;
Expand All @@ -186,6 +191,7 @@ export class String {

@operator("<=")
private static __lte(left: String, right: String): bool {
if (left === right) return true;
if (left === null) return right === null;
else if (right === null) return false;
Copy link
Member

Choose a reason for hiding this comment

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

Can remove the else

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed


Expand Down Expand Up @@ -377,6 +383,40 @@ export class String {
);
return out;
}

repeat(count: i32 = 0): String {
assert(this !== null);
var length = this.length;

// Most browsers can't handle strings 1 << 28 chars or longer
if (count < 0 || length * count > (1 << 28)) {
throw new RangeError("Invalid count value");
}

if (count === 0 || !length) return EMPTY;
if (count === 1) return this;

var result = allocate(length * count);
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this missing the leading 32-bit length?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm. I use this allocation method which already include HEADER size during allocation if I right understand your comment =)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yeah, makes sense then.

var strLen = length << 1;

/*
* TODO possible improvments: reuse existing result for exponentially concats like:
* 'a' + 'a' => 'aa' + 'aa' => 'aaaa' + 'aaaa' etc
*/
for (let offset = 0, len = strLen * count; offset < len; offset += strLen) {
move_memory(
Copy link
Member

Choose a reason for hiding this comment

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

I guess copy size could also increase. Like, repeating "a" four times would first create "aa" and then copy "aa" to "aaaa". Wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about it, and move_memory may better optimize for copy large world chunks, but this increase codebase and but benefits may be not so huge. Need benchmark. For now I can add TODO comment for future if don't mind

changetype<usize>(result) + HEADER_SIZE + offset,
changetype<usize>(this) + HEADER_SIZE,
strLen
);
}

return result;
}

toString(): String {
return this;
}
}

export function parseInt(str: String, radix: i32 = 0): f64 {
Expand Down
5 changes: 5 additions & 0 deletions std/portable.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,16 @@ declare class Float32Array extends Array<f32> {}
declare class Float64Array extends Array<f64> {}

declare class String {

static fromCharCode(ls: i32, hs?: i32): string;
static fromCharCodes(arr: u16[]): string;
static fromCodePoint(cp: i32): string;
static fromCodePoints(arr: i32[]): string;

readonly length: i32;

private constructor();

indexOf(subject: string, position?: i32): i32;
includes(other: string): bool;
lastIndexOf(subject: string, position?: i32): i32;
Expand All @@ -262,6 +266,7 @@ declare class String {
startsWith(subject: string): bool;
endsWith(subject: string): bool;
replace(search: string, replacement: string): string;
repeat(count?: i32): string;
toString(): string;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/compiler/std/array-access.optimized.wat
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
(call $abort
(i32.const 0)
(i32.const 8)
(i32.const 232)
(i32.const 238)
(i32.const 4)
)
(unreachable)
Expand Down
2 changes: 1 addition & 1 deletion tests/compiler/std/array-access.untouched.wat
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
(call $abort
(i32.const 0)
(i32.const 8)
(i32.const 232)
(i32.const 238)
(i32.const 4)
)
(unreachable)
Expand Down
Loading