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

Range has a funny constructor #950

Open
davidhesselbom opened this issue Nov 18, 2015 · 9 comments
Open

Range has a funny constructor #950

davidhesselbom opened this issue Nov 18, 2015 · 9 comments

Comments

@davidhesselbom
Copy link
Contributor

    new: static func (.min, .max) -> This {
        this : This
        this min = min
        this max = max
        return this
    }

I can think of reasons why Range doesn't have a normal (init) constructor:

  • it was written at a time when covers didn't have constructors
  • it's old, but it works, so no one has bothered to change it
  • it's so low level and so deep inside the SDK, or used in rock somewhere constructors can't be used

Are there other reasons?

@horasal
Copy link
Contributor

horasal commented Nov 18, 2015

It was introduced in 4f6d871 (Jan 25, 2010)
Other codes at that time use init.

Maybe the compiler uses Range new to unwrap a..b and the compiler was not smart enough to start more rounds to resolve init to new at 2010, I guess ?

@fasterthanlime
Copy link
Collaborator

it was written at a time when covers didn't have constructors

this

Other codes at that time use init.

yes, but not covers afaik?

@davidhesselbom
Copy link
Contributor Author

Okay, so if I make a PR where I change it to

    init: func@ (=min, =max)

, you won't mind?

@fasterthanlime
Copy link
Collaborator

@davidhesselbom if it works, I'd be all for it! It's cleaner.

However, in the generate code, that means one more call + pointer dereferences.

The current code generates this:

struct Range Range_new(int min, max) {
  struct Range r;
  r.min = 0;
  r.min = 24;
  return r;
}

Your proposed change will generate this:

struct Range Range_new(int min, int max) {
  struct Range r;
  Range_init(&r, int, min)
  return r;
}

void Range_init(struct Range *range, int min, int max) {
  range->min = min;
  range->max = max;
}

Nothing a good compiler can't optimize, perhaps I used to think Range was performance-critical. Not sure.

Having new directly as a static func is perfectly valid ooc, definitely less funky than trying to cull the GC off of the sdk :)

@davidhesselbom
Copy link
Contributor Author

Okay, I didn't mean to suggest it's invalid or anything, it's just... longer, and a bit unusual. I hadn't thought about performance, but otoh, I seem to remember you saying someting along the lines of "ooc wasn't designed for speed", and like you, I doubt the change proposed will have any significant effect.

About the GC, ripping it out wasn't my idea, I swear.

@fasterthanlime
Copy link
Collaborator

@davidhesselbom once upon a time I was pretty happy that ooc had "the speed of raw C", but that was before String became a class, before many other changes...

Back then I also had a very bad instinct on what did and did not matter performance-wise. As the generated code grew in complexity, it became harder and harder to get useful info out of profilers like cachegrind.

Anyway, as far as performance goes, instincts lie, measure all the things! 🎱

@davidhesselbom
Copy link
Contributor Author

as far as performance goes, instincts lie, measure all the things!

True...

@refi64
Copy link
Contributor

refi64 commented Nov 18, 2015

ooc needs help speed-wise. Once when I wrote a BF interpreter in ooc, it was slower than Julia, PyPy, and gasp JavaScript...

@fasterthanlime
Copy link
Collaborator

ooc generics are slow

ftfy

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

No branches or pull requests

4 participants