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

Clean up the Perlin noise benchmark #12270

Merged
merged 1 commit into from
Feb 15, 2014
Merged

Clean up the Perlin noise benchmark #12270

merged 1 commit into from
Feb 15, 2014

Conversation

bstrie
Copy link
Contributor

@bstrie bstrie commented Feb 14, 2014

Mostly just style fixes, but also remove a heap allocation and switch to using a buffered writer rather than doing 60,000 println!s.

@alexcrichton
Copy link
Member

All the style fixes look great to me, but did you see a perf improvement when siwtching from println! to BufferedWriter? When using println! you're using a LineBufferedWriter, so I wouldn't expect the performance to increase too much. Stylistically-wise, I prefer using println!, but if you saw a good deal of perf gain it's fine.

permutations: permutations,
}
let mut rgradients = [Vec2 { x: 0.0, y: 0.0 }, ..256];
for i in range(0, 256) { rgradients[i] = random_gradient(&mut rng); }
Copy link
Member

Choose a reason for hiding this comment

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

Why not iterators? for x in rgradients.mut_iter() { *x = random_gradient(&mut rng); }

@bstrie
Copy link
Contributor Author

bstrie commented Feb 15, 2014

@alexcrichton Although this version does run faster than the prior one, it's very slight... only two hundredths of a second faster, on average, on a machine that is pretty slow to begin with. I had no idea that println! was buffered, I do much prefer the look of it, so I'll make that change.

@bstrie
Copy link
Contributor Author

bstrie commented Feb 15, 2014

Actually, the switch from range to mut_iter looked totally fine once I started being willing to split the loop across more than one line. r?

@@ -8,128 +8,112 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Perlin noise benchmark from https://gist.github.com/1170424
// Multi-language Perlin noise benchmark.
Copy link
Member

Choose a reason for hiding this comment

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

Multi-language?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check out the link in the next line, there's D, Nimrod, Go, C, C#, lots more. Lots of the structure of this code is the way it is in order to remain easily comparable to these other implementations.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, cool!

And we're winning \o/

@huonw
Copy link
Member

huonw commented Feb 15, 2014

r=me after squashing

bors added a commit that referenced this pull request Feb 15, 2014
Mostly just style fixes, but also remove a heap allocation and switch to using a buffered writer rather than doing 60,000 `println!`s.
@bors bors closed this Feb 15, 2014
@bors bors merged commit bfa3e60 into rust-lang:master Feb 15, 2014
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 13, 2024
`lint_groups_priority`: ignore lints & groups at the same level

Fixes rust-lang#12270

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants