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

Stress test #1271

Merged
merged 7 commits into from Oct 8, 2019
Merged

Stress test #1271

merged 7 commits into from Oct 8, 2019

Conversation

tsherif
Copy link
Contributor

@tsherif tsherif commented Oct 4, 2019

Render 400,000 cubes in 4000 draw calls and report CPU and GPU frame time. So I can track perf improvements over the course of the refactor.

@coveralls
Copy link

coveralls commented Oct 4, 2019

Coverage Status

Coverage remained the same at 63.445% when pulling e0bf7e4 on stress-test into 75ab19e on master.

@tsherif tsherif added this to In progress in Milestone 8.0 Oct 7, 2019
Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

Nice!

Just as an idea, I think having the option of running a single draw call with 400K and contrasting it with 1000 draw calls of 400 would really cast a light on the overhead we are adding.

Could be a query param or a toggle button...

CubeGeometry
} from '@luma.gl/core';
import {Matrix4, radians} from 'math.gl';
/* eslint-disable spaced-comment, max-depth */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nits:

  • Usually place linter "imports". i.e. /* global .. */ above the import section.
  • When possible, please use // eslint-disable-line rule or // eslint-disable-next-line rule to minimize the amount of code affected by the disable. May not be practical for the spaced-comment but should be used for the max-depth.

/* eslint-disable spaced-comment, max-depth */
/* global document, window */

/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Attributions go at the very top in most files in other repos

const matrices = new Float32Array(count * 16);
const matrixBuffer = new Buffer(gl, matrices.byteLength);

const vs = `\
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Recommend defining these as global constants above the constructor, to make the JS code here more readable.


constructor(props = {}) {
super(props);
// Default value is true, so GL context is always created to verify wheter it is WebGL2 or not.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: whether

depthFunc: GL.LEQUAL
});

const projMat = new Matrix4();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: type out variable names


const {gl, aspect, projMat, viewMat, instancedCubes, angle} = props;

cpuStats.innerHTML = `CPU Time: ${this.stats
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use StatsWidget?

@tsherif tsherif mentioned this pull request Oct 7, 2019
@tsherif
Copy link
Contributor Author

tsherif commented Oct 8, 2019

I think it would be great to think though ways to manipulate this scene (or completely different scenes) to get different kinds of information. It is pretty limited in the parts of the system it hits. For this first pass, though, I just wanted to get something up and running quickly that runs heavy on both the CPU and GPU.

@tsherif tsherif merged commit 25bccf7 into master Oct 8, 2019
@tsherif tsherif deleted the stress-test branch October 8, 2019 21:22
@tsherif tsherif moved this from In progress to Done in Milestone 8.0 Oct 8, 2019
@tgorkin tgorkin added this to the 8.0 milestone Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Milestone 8.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants