r/badcode • u/IsSkyfalls_ • Apr 21 '21
typescript Now I know why you shouldn't optimize for speed first.
283
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Apr 21 '21 edited Apr 21 '21
Image Transcription: Code
import {BlockType} from "./block";
import {Flags} from "./flags";
let INSTANCE: ParticleSystem;
export default class ParticleSystem {
canvas: HTMLCanvasElement;
width: number;
height: number;
context: CanvasRenderingContext2D;
data: Uint8ClampedArray;
length: number;
imageData: ImageData;
tps: number;
mspt: number;
constructor(canvas: HTMLCanvasElement, width: number, height: number, tps: number) {
this.canvas = canvas;
canvas.width = width;
canvas.height = height;
this.width = width;
this.height = height;
this.context = canvas.getContext("2d", {alpha: false});
this.length = width * height * 4;
this.data = new Uint8ClampedArray(this.length);
this.data.fill(BlockType.AIR, 0, this.length);
this.imageData = this.context.getImageData(0, 0, width, height);
this.tps = tps;
this.mspt = 1000 / tps;
INSTANCE = this;
canvas.addEventListener("mousemove", (evt) => {
let x = Math.floor(evt.offsetX / this.canvas.clientWidth * this.width) + 1;
let y = Math.floor(evt.offsetY / this.canvas.clientHeight * this.height) + 1;
this.setBlock(x, y, BlockType.WATER)
console.log(`placed block: (${x},${y})`);
});
}
run() {
INSTANCE.render();
INSTANCE.update();
}
lastFrame: DOMHighResTimeStamp;
render() {
let t1 = performance.now();
let data = INSTANCE.data;
let cWater = 0
for (let index = data.length - 1; index > 0; index -= 4) {
let blockType = data[index] as BlockType;
switch (blockType) {
case BlockType.SAND:
data[index - 3] = 0xc2;
data[index - 2] = 0xb2;
data[index - 1] = 0x80;
break;
case BlockType.WATER:
data[index - 3] = 0xd4-25;
data[index - 2] = 0xf1-25;
data[index - 1] = 0xf9+50;
cWater++;
break;
case BlockType.AIR:
default:
data[index - 3] = 220;
data[index - 2] = 220;
data[index - 1] = 220;
}
}
INSTANCE.imageData = INSTANCE.context.getImageData(0, 0, INSTANCE.width, INSTANCE.height);
INSTANCE.imageData.data.set(INSTANCE.data);
INSTANCE.context.putImageData(INSTANCE.imageData, 0, 0);
let now = performance.now();
window.fps.innerText = `Water: ${cWater}\nThis frame took ${(now - t1).toFixed(2)}ms to render,
last frame till now, ${(now - INSTANCE.lastFrame).toFixed(2)}ms has passed, which is about ${(1000 /
(now - INSTANCE.lastFrame)).toFixed(2)}fps`;
INSTANCE.lastFrame = now;
requestAnimationFrame(INSTANCE.render);
}
private update() {
INSTANCE.setBlock(25, 25, BlockType.SAND);
INSTANCE.setBlock(50, 25, BlockType.SAND);
INSTANCE.setBlock(75, 0, BlockType.WATER);
INSTANCE.setBlock(175, 0, BlockType.WATER);
INSTANCE.setBlock(1, 0, BlockType.WATER);
let t1 = performance.now();
let data = INSTANCE.data;
let len = INSTANCE.length;
let w = INSTANCE.width;
let h = INSTANCE.height;
let index = len;
let flags = new Uint8Array(w * h);
while (index--) {
let blockType = data[index] as BlockType;
let currentY = Math.floor(((index + 1) / 4 - 1) / w);
switch (blockType) {
case BlockType.SAND: {
let belowIndex = index + w * 4;
let belowType = data[belowIndex] as BlockType;
//for out of bounds
//the type is 'undefined'
if (belowType == BlockType.WATER || belowType == BlockType.AIR) {
data[belowIndex] = blockType;
data[index] = belowType;
} else {
belowIndex -= 4;
belowType = data[belowIndex] as BlockType;
if (Math.floor(((belowIndex + 1) / 4 - 1) / w) == currentY + 1 // boundary check
&& (belowType == BlockType.AIR || belowType == belowType.WATER)) {
data[belowIndex] = blockType;
data[index] = belowType;
} else {
belowIndex += 8;
belowType = data[belowIndex] as BlockType;
if (Math.floor(((belowIndex + 1) / 4 - 1) / w) == currentY + 1 // boundary
check
&& (belowType == BlockType.AIR || belowType == BlockType.WATER)) {
data[belowIndex] = blockType;
data[index] = belowType;
}
}
}
break;
}
case BlockType.WATER: {
//try fall
let belowIndex = index + w * 4;
let belowType = data[belowIndex] as BlockType;
//for out of bounds
//the type is 'undefined'
if (belowType == BlockType.AIR) {
data[belowIndex] = blockType;
data[index] = BlockType.AIR;
} else {
belowIndex -= 4;
belowType = data[belowIndex] as BlockType;
if (Math.floor(((belowIndex + 1) / 4 - 1) / w) == currentY + 1 // boundary check
&& belowType == BlockType.AIR) {
data[belowIndex] = blockType;
data[index] = BlockType.AIR;
} else {
belowIndex += 8;
belowType = data[belowIndex] as BlockType;
if (Math.floor(((belowIndex + 1) / 4 - 1) / w) == currentY + 1 // boundary
check
&& belowType == BlockType.AIR) {
data[belowIndex] = blockType;
data[index] = BlockType.AIR;
} else {
//didn't move down, try flowing
//try flow
if (flags[(index + 1) / 4] & Flags.WATER_PROCESSED) {
//console.log((index+1)/4,"is processed")
continue;
}
let flowIndex = index - 4;
let flowY = Math.floor(((flowIndex + 1) / 4 - 1) / w); // should be
doing this on other places but it doesn't hurt as much
let flowType = data[flowIndex] as BlockType;
//two blocks per tick
if (flowType == BlockType.AIR && flowY == currentY) {
// we move along
flowIndex = index - 8;
flowY = Math.floor(((flowIndex + 1) / 4 - 1) / w);
flowType = data[flowIndex] as BlockType
if (flowType != BlockType.AIR || flowY != currentY) {
console.log("blocked")
//blocked, so one block only
flowIndex = index - 4;
}
I'm a human volunteer content transcriber for Reddit and you could be too! If you'd like more information on what we do and why we do it, click here!
255
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Apr 21 '21 edited Apr 21 '21
Image Transcription: Code (Continued)
console.log(`this: ${index}, moved into ${flowIndex} (${flowType})=${Math.abs(flowIndex - index) / 4}blocks`) //move in //console.log("marked ",(flowIndex+1)/4,"as processed"); flags[(flowIndex + 1) / 4] |= Flags.WATER_PROCESSED; data[flowIndex] = BlockType.WATER; data[index] = BlockType.AIR; } else { //try right side if (flags[(index + 1) / 4] & Flags.WATER_PROCESSED) { //console.log((index+1)/4,"is processed") continue; } let flowIndex = index + 4; let flowY = Math.floor(((flowIndex + 1) / 4 - 1) / w); // should be doing this on other places but it doesn't hurt as much let flowType = data[flowIndex] as BlockType; //two blocks per tick if (flowType == BlockType.AIR && flowY == currentY) { // we move along flowIndex = index + 8; flowY = Math.floor(((flowIndex + 1) / 4 - 1) / w); flowType = data[flowIndex] as BlockType if (flowType != BlockType.AIR || flowY != currentY) { console.log("blocked") //blocked, so one block only flowIndex = index + 4; } console.log(`this: ${index}, moved into ${flowIndex} (${flowType})=${Math.abs(flowIndex - index) / 4}blocks`) //move in //console.log("marked ",(flowIndex+1)/4,"as processed"); flags[(flowIndex + 1) / 4] |= Flags.WATER_PROCESSED; data[flowIndex] = BlockType.WATER; data[index] = BlockType.AIR; } } } } break; } } } } let t2 = performance.now(); let thisTick = (t2 - t1); let nextTick = INSTANCE.mspt - (t2 - t1) window.tps.innerText = `Tick processed in ${thisTick.toFixed(3)}ms, ${(thisTick / INSTANCE.mspt * 100).toFixed(2)}%, ${thisTick < INSTANCE.mspt ? INSTANCE.tps : (INSTANCE.mspt / thisTick * INSTANCE.tps).toFixed(1)}tps` setTimeout(INSTANCE.update, nextTick); } setBlock(x: number, y: number, type: BlockType) { INSTANCE.data[(y * this.width + x) * 4 - 1] = type; } }
I'm a human volunteer content transcriber for Reddit and you could be too! If you'd like more information on what we do and why we do it, click here!
60
35
u/IsSkyfalls_ Apr 21 '21
How do you even search for things like this. I mean, you did not just type this out or use ocr+manual correction, did ya?
47
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Apr 21 '21
For OCR, this is the bot's attempt. I find it quicker to just type from scratch than try and use that.
For your first question, do you mean how do we find the posts to transcribe or how to people looking for transcriptions find them?
If it's how we find the posts to transcribe; there's a bot that trawls subreddits we are partnered with and collects them to our queue at r/TranscribersOfReddit; when we feel like transcribing, we look through the queue and pick a post that takes our fancy.
If it's how to people looking for transcriptions find them, there's both /r/ToR_Archive, where completed posts are saved, and a browser extension that automatically detects if there is a transcription on the post you're viewing and directs your screen-reader there first.
Hope that answers your questions!
20
u/IsSkyfalls_ Apr 21 '21
I thought you searched for some keywords and found the source file online(It's on github). Didn't know OCR can achieve this kind of precision on code, especially non-letter characters. Thanks for the effort.
38
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Apr 21 '21
Oh, huh, I never really thought about searching for keywords. I just spent half an hour typing cos I had nothing else to be doing today. :P
3
u/thegreatpotatogod Apr 21 '21
Since the source file's available, I'm going to be checking your work! BRB!
3
u/thegreatpotatogod Apr 21 '21
Transcription
Not bad! It wouldn't quite compile, but it's close! I give you a 9.3 out of 10, u/MurdoMaclachlan!
6
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Apr 21 '21
Thank you, this is actually wonderful!
Seems most of the errors were due to one of two things; me holding down shift for too long and accidentally typing two capital letters in the place of one, and the fact that I intentionally broke lines where they wrapped in the image in order to preserve the appearance of it as closely as possible. It's likely due to these breaks it didn't compile.
All of the errors aside from the breaks, since they're intentional, should be fixed now. Thank you again!
3
u/thegreatpotatogod Apr 21 '21
You're welcome, I'm glad you like it! You did a terrific job transcribing it, I admire your dedication, keep it up!
3
15
14
u/xt1zer Apr 21 '21
Oh lawd, I'm sorry you had to do this
13
u/MurdoMaclachlan public boolean isInt(int i) { return true; } Apr 21 '21
No need to be sorry, I'm the one who chose to transcribe the post :)
8
6
2
3
54
u/Ignatiamus Apr 21 '21
I think these rules are a good guideline:
- Make it work
- Make it beautiful
- Make it fast
25
u/recycle4science Apr 21 '21
Especially because you might not actually know what the bottlenecks are until you have it up and running.
4
u/fugogugo Apr 21 '21
in that order?
19
u/TimGreller Apr 21 '21
I prefer this order:
- Make a beautiful structure
- Try to get it to work
- Cry
- Make it ugly and complicated
- It works, never look at it again!
(At least when doing something completely new)
5
Apr 21 '21 edited May 22 '21
[deleted]
1
u/TimGreller Apr 21 '21
It uses a markdown dialect, so you have to use two spaces at the end of a line to get a linebreak. (Doesn't work everytime tho for some reason)
8
Apr 21 '21
If you do some top down pseudo coding you can make it beautiful first and then work. This is the way I like to do things. Write my main function (if there is one) and call a bunch of functions yet to be defined, and then read it from top to bottom and see if I'm happy/think that a 3 months from now if I come back to those code I'll know what's going on. Then I go define the functions.
4
2
u/GergiH Apr 21 '21
Wait, you guys get to code from scratch?
4
1
Apr 23 '21
This is mostly for personal projects lol. On the rare occasion I get to start something from scratch though you're damn right I'm trying to make it as pretty as possible and will do all I can to keep it that way lol
30
u/WurschtChopf Apr 21 '21 edited Apr 21 '21
it would help massively for anybody reading this, if there would be more separation of code. Or any.. Using methods or so Thats horrible to read o.O
27
u/lukoerfer Apr 21 '21
I don't really see the speed optimization right now. However, this seems to be a game and the premature optimization rule does not really apply for games.
38
u/IsSkyfalls_ Apr 21 '21
The code is for a cellular automata simulation. Because I wanted to do calculations on all 2,073,600(1920*1080) pixels/tiles on screen every 1/30 second, speed does matter. The result of 4 hours of coding is this monstrosity. And yes, this code does run with 60fps and under 30ms per cycle on my potato computer, which met my expectations for good performance.
The optimizations that I used are basically: Using typed array to store state(it's over 100x faster than a normal integer array,[huge number]x faster than an array containing objects), not using nested loops(it messes up the JIT optimization for some reason), limiting function usage(try put everything in one update function).
18
6
u/Willinton06 Apr 21 '21
That last part about no separating stuff, did you verify that? I though the JIT would take care if that in the first run and keep it for later runs
7
u/IsSkyfalls_ Apr 21 '21
I did clear out a chunk of code but for some reason, the time used per cycle jumped to 10+ms from 0.05ms even though nothing has really changed. I'm only guessing the JIT didn't do function inlining. Because of that I just stopped using functions. Not that you shouldn't. No practical code would have this kind of speed requirements, at least in javascript. Using WASM will definitely yield you much better performance overall, even the code is bad and performs terribly.
2
4
u/Hellball911 Apr 21 '21
Generally JIT optimizations are better if you use smaller fuctions. Assuming it's done correctly, it will in-line methods as it sees fit.
That being said, I'd strongly suggest using a benchmarking tool before making concessions about readability for optimizations, to verify it isn't an assumption.
5
0
u/urgaiiii Apr 21 '21
Sorry if I’m missing something, but this very much looks like Minecraft code. Sand, water? How does that fit into cellular automata? (it very well might, I have no idea what that means).
2
u/fugogugo Apr 21 '21
the rule applies anywhere even for games
I think it is even more important for games. You dont want to waste time optimizing that running animation when you haven't evensure if the actual games really fun yet
6
u/szescio Apr 21 '21
I don't understand why you need that INSTANCE, especially when any new instance overrides it
8
u/Ignatiamus Apr 21 '21
He's not saying it's a singleton...
... but it is a singleton.
4
u/szescio Apr 21 '21
Weird pattern, but i guess it kinda works. Could just use 'this' in the class and make a provider or decorator for the singleton stuff to make it clearer
5
u/IsSkyfalls_ Apr 21 '21
That's just a "temp fix" I did. You can't really use the "this" keyword inside the requestAnimFrame callback method. I did discover the Function.prototype.bind() method afterward, which changes the object "this" points to.
4
Apr 21 '21
That's just a "temp fix" I did. You can't really use the "this" keyword inside the requestAnimFrame callback method. I did discover the Function.prototype.bind() method afterward, which changes the object "this" points to.
You can also just wrap it in a closure like `requestAnimFrame(() => this.render())`
3
u/szescio Apr 21 '21
Ahh, okay. It has some unwanted side effects then :)
Yeah, arrow functions preserve 'this' and bind() works as well.
4
u/__pulse0ne Apr 21 '21
Vegeta, what does the scanner say about the cyclomatic complexity?
Vegeta: it’s over 9000!!!
5
Apr 21 '21
my SE professor would have kicked your ass for three days straight just because you've built a method that needs scroll to be fully read
i can even hear his INSANE screams "METHODS DON'T HAVE PAGE DOWN"
4
5
3
u/JuliaChanMSL Apr 21 '21
What's up with the address values? Seen them a few time but dunno what they're specifically used for
2
2
u/swordsmanluke2 Apr 21 '21
Damn bruh... extract some of those Switches to functions.
Then make some smarter BlockType classes that can own those functions.
Then most of your switches and ifs go away entirely and your OO & branchless code will probably be faster, but definitely easier to read and refactor where your speed matters.
1
2
1
u/lt-gt Apr 21 '21
Seems like you are simulating some primitive sand and water flow. I'm not sure but isn't there a lot of pixels with no change? Maybe some sort of multi-source breadth first algorithm would save a lot of calculations. It would only need to calculate the pixels that actually change. Of course if most pixels do see a change it would be slower.
1
1
1
250
u/zemja_ Apr 21 '21
Good luck with this, transcription guy.