Add String#repeat method #67
Conversation
This reverts commit 1bc1f6b.
| Ready for merging |
std/assembly/string.ts Outdated
| | ||
| repeat(count: i32 = 0): String { | ||
| assert(this !== null); | ||
| assert(count >= 0); |
There was a problem hiding this comment.
I believe JS does throw new RangeError("Invalid count value") there, hmm. Still unsure what's best as this will eventually result in quite a few static strings in modules where methods that do so are used. For now I'd tend to stay consistent with JS here, as other functions do it as well.
std/assembly/string.ts Outdated
| | ||
| while (count) { | ||
| if (count & 1) result = result.concat(str); | ||
| if (count > 1) str = str.concat(str); |
There was a problem hiding this comment.
I wonder if there'd be a benefit in doing this on a lower level, like inlined. Could potentially reduce allocations in that a properly sized buffer can be pre-allocated. Wdyt?
| if (count === 0 || !length) return EMPTY; | ||
| if (count === 1) return this; | ||
| | ||
| var result = allocate(length * count); |
There was a problem hiding this comment.
Isn't this missing the leading 32-bit length?
There was a problem hiding this comment.
Hmm. I use this allocation method which already include HEADER size during allocation if I right understand your comment =)
| var strLen = length << 1; | ||
| | ||
| for (let offset = 0, len = strLen * count; offset < len; offset += strLen) { | ||
| move_memory( |
There was a problem hiding this comment.
I guess copy size could also increase. Like, repeating "a" four times would first create "aa" and then copy "aa" to "aaaa". Wdyt?
There was a problem hiding this comment.
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
std/assembly/string.ts Outdated
| 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; |
std/assembly/string.ts Outdated
| if (left === null || right === null) return false; | ||
| if (left === right || left === null || right === null) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
So I should refactor this to 3-timed if instead one block?
There was a problem hiding this comment.
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 There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
| Thanks! :) |
2 participants
No description provided.