7 ms·

That comment is not the reason for the function’s existence, but the details of how the function interacts with GC. The reason for the function’s existence is

That comment is not the reason for the function’s existence, but the details of how the function interacts with GC. The reason for the function’s existence is that it allows typed arrays to dynamically switch between a fast/compact representation for the case that the JSVM owns the data, and a slightly slower and slightly less compact version for when the JSVM allows native code to share ownership of the data. This function, slowDownAndWasteMemory, switches to the less efficient version that allows aliasing with native code. Of course the name is sarcastic. The actual effect of having this is that JSC can handle both the owned case and the aliased case, and get a small will if you’re in the owned case while being able to switch to the aliased case at any time. Since there’s no way to support the aliased case without some slightly higher memory/time usage, we are sort of literally slowing down and wasting memory when we go aliased. Source: I’m pretty sure I wrote most of this code.

londons_explore 3y ago
Indeed you did, 10 years ago. This is the (mega!) commit: https://github.com/WebKit/WebKit/commit/93a48aa964f25ce8ec9f90aeaa142d2a9fb5b678 https://github.com/WebKit/WebKit/commit/93a48aa964f25ce8ec9f... I often wonder how much more productive I could be if I didn't need to split changes like this into 50 small commits, where every intermediate commit is working and has passing tests...
- pizlonator 3y ago
  Yeah sometimes you just have to write a megapatch. I’d be very sad if I had to work on a project that didn’t allow that.
  - bpye 3y ago
    I don't know what my largest commit has been, but I do find that merge conflicts end up being a real pain when you're touching so many files...
    - AlotOfReading 3y ago
      Mine was a very memorable ~2900 files changed. We missed an OS update cycle and had to consume all the changes in one giant patch. But upstream had migrated from Python 2 to 3 in-between and rewrote half their build scripts to accommodate. Each of those changes needed to be reviewed manually because we had modified things to support reproducible builds and the resulting merge conflict was monstrous. I contributed maybe 2300 of those before my sanity failed and called for help. Now we have a whole team to do that job more regularly.
  - phatskat 3y ago
    I understand the desire to make larger patches, but how do you effectively manage them in the review process? For super large commits in the past, I’ve had other engineers hop on a screen share to review the diff together and answer questions, but it feels inefficient.
    - 8n4vidtmkvmk 3y ago
      Y'all are very thorough. Just make sure the function has tests and won't blow up prod. You needn't waste forever reviewing. It's code. It can be iterated upon.
      - londons_explore 3y ago
        Indeed. I always push my teams into not hunting for spelling mistakes. Instead look for things that will be hard to fix in the future ('did you realise that this bootloader config stops us doing field updates??') or where the overall direction is wrong ('Not sure about os.system('grep') for this XML - why can't we just use the xml.etree here?').
        js2 3y ago
        > not hunting for spelling mistakes Unless it's the name of an HTTP header?
        ianlevesque 3y ago
        Not sure what you’re refering to.
        missblit 3y ago
        Referer https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referer https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Re...
        deleted 3y ago
- kccqzy 3y ago
  I find this to be more of a reflection of how well aligned you and the code reviewer are. When both of you know the code base inside out and trust each other, it's fine. But sometimes the author is a noob and the code reviewer has to review every little detail, much like an examiner looking at a student's paper. Or sometimes they might have a personal beef.
  - hgsgm 3y ago
    I don't want a personal review, I want a code reviewer. "Trusting" someone means normalization of deviance, where bugs slip in.
- eru 3y ago
  > I often wonder how much more productive I could be if I didn't need to split changes like this into 50 small commits, where every intermediate commit is working and has passing tests... This is down to a trade-off between ease of writing and ease of review. Yes, if you didn't have to make your work presentable, you could bang it out faster. But then no one could read it.
qingcharles 3y ago
This is classic HN. "Look at this crazy, obscure code everybody!" *ORIGINAL DEVELOPER APPEARS*
- astrange 3y ago
  Obscurity is mostly a property of the project it's in, so I don't know if it can really apply to WebKit.
kentonv 3y ago
Out of curiosity, what makes the aliased version less compact? Naively I'd think there's not much room to optimize the representation of a byte array.
- pizlonator 3y ago
  Native code wants to think it’s a ref counted buffer that could be accessed from any thread including ones that don’t participate in GC. So it’s gotta be allocated in the malloc heap and it’s gotta have a ref count. JS code wants the buffer to be garbage collected. If native code doesn’t ref it (count is zero) and it’s not reachable via JS heap then it’s gotta die. So, it needs a GC cell and associated machinery (JSC object headers, mark bits, possibly other stuff). So, the reason why the aliased case is not as awesome as the owned case is that you need double the things. You need some malloc memory and you need a GC cell. You need a ref count and you need a GC header and some mark bits. Having both malloc/RC and cell/GC overhead isn’t like the end of the world or anything, but it means that allocations are slower and waste more memory than if it was either just a buffer owned by native code or just an object in the GC heap.
solarkraft 3y ago
Did you choose the name? Have there been any reactions to it? I like the name, but can easily imagine somebody preferring a more serious alternative.
- pizlonator 3y ago
  That’s why it’s important to find a team and coworkers that aren’t so uptight.
  - slaymaker1907 3y ago
    With a project that could live on for many years, I’d object to this code, but only because it doesn’t really explain _why_ it’s necessary despite wasting time/memory. The humorous name is great, though. It might be explained in the VCS history, but people aren’t likely to look it up there and I’ve noticed the history tends to get lost for a lot of old projects.
    - eru 3y ago
      Fortunately, git helps you stitch together histories (without changing hashes), if you later recover it: https://git-scm.com/book/en/v2/Git-Tools-Replace https://git-scm.com/book/en/v2/Git-Tools-Replace
    - pizlonator 3y ago
      The name isn’t just funny. It’s also accurate. And it’s quite evidently stood the test of time. In a VM, often the most important kind of documentation is about labeling what is the fast path and what is the slow path. This method name alerts the code reader that they’re looking at one of the slow paths.
  - spopejoy 3y ago
    Isn't part of the goal with deliberately weird names like this is to alert callers that HERE BE DRAGONS so, like, don't call this unless you've read _and understood_ the source code?

londons_explore 3y ago

Indeed you did, 10 years ago. This is the (mega!) commit: https://github.com/WebKit/WebKit/commit/93a48aa964f25ce8ec9f90aeaa142d2a9fb5b678 https://github.com/WebKit/WebKit/commit/93a48aa964f25ce8ec9f... I often wonder how much more productive I could be if I didn't need to split changes like this into 50 small commits, where every intermediate commit is working and has passing tests...

pizlonator 3y ago
Yeah sometimes you just have to write a megapatch. I’d be very sad if I had to work on a project that didn’t allow that.
- bpye 3y ago
  I don't know what my largest commit has been, but I do find that merge conflicts end up being a real pain when you're touching so many files...
  - AlotOfReading 3y ago
    Mine was a very memorable ~2900 files changed. We missed an OS update cycle and had to consume all the changes in one giant patch. But upstream had migrated from Python 2 to 3 in-between and rewrote half their build scripts to accommodate. Each of those changes needed to be reviewed manually because we had modified things to support reproducible builds and the resulting merge conflict was monstrous. I contributed maybe 2300 of those before my sanity failed and called for help. Now we have a whole team to do that job more regularly.
- phatskat 3y ago
  I understand the desire to make larger patches, but how do you effectively manage them in the review process? For super large commits in the past, I’ve had other engineers hop on a screen share to review the diff together and answer questions, but it feels inefficient.
  - 8n4vidtmkvmk 3y ago
    Y'all are very thorough. Just make sure the function has tests and won't blow up prod. You needn't waste forever reviewing. It's code. It can be iterated upon.
    - londons_explore 3y ago
      Indeed. I always push my teams into not hunting for spelling mistakes. Instead look for things that will be hard to fix in the future ('did you realise that this bootloader config stops us doing field updates??') or where the overall direction is wrong ('Not sure about os.system('grep') for this XML - why can't we just use the xml.etree here?').
      - js2 3y ago
        > not hunting for spelling mistakes Unless it's the name of an HTTP header?
        ianlevesque 3y ago
        Not sure what you’re refering to.
        missblit 3y ago
        Referer https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Referer https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Re...
        deleted 3y ago
kccqzy 3y ago
I find this to be more of a reflection of how well aligned you and the code reviewer are. When both of you know the code base inside out and trust each other, it's fine. But sometimes the author is a noob and the code reviewer has to review every little detail, much like an examiner looking at a student's paper. Or sometimes they might have a personal beef.
- hgsgm 3y ago
  I don't want a personal review, I want a code reviewer. "Trusting" someone means normalization of deviance, where bugs slip in.
eru 3y ago
> I often wonder how much more productive I could be if I didn't need to split changes like this into 50 small commits, where every intermediate commit is working and has passing tests... This is down to a trade-off between ease of writing and ease of review. Yes, if you didn't have to make your work presentable, you could bang it out faster. But then no one could read it.

qingcharles 3y ago

This is classic HN. "Look at this crazy, obscure code everybody!" *ORIGINAL DEVELOPER APPEARS*

astrange 3y ago
Obscurity is mostly a property of the project it's in, so I don't know if it can really apply to WebKit.

kentonv 3y ago

Out of curiosity, what makes the aliased version less compact? Naively I'd think there's not much room to optimize the representation of a byte array.

pizlonator 3y ago
Native code wants to think it’s a ref counted buffer that could be accessed from any thread including ones that don’t participate in GC. So it’s gotta be allocated in the malloc heap and it’s gotta have a ref count. JS code wants the buffer to be garbage collected. If native code doesn’t ref it (count is zero) and it’s not reachable via JS heap then it’s gotta die. So, it needs a GC cell and associated machinery (JSC object headers, mark bits, possibly other stuff). So, the reason why the aliased case is not as awesome as the owned case is that you need double the things. You need some malloc memory and you need a GC cell. You need a ref count and you need a GC header and some mark bits. Having both malloc/RC and cell/GC overhead isn’t like the end of the world or anything, but it means that allocations are slower and waste more memory than if it was either just a buffer owned by native code or just an object in the GC heap.

solarkraft 3y ago

Did you choose the name? Have there been any reactions to it? I like the name, but can easily imagine somebody preferring a more serious alternative.

pizlonator 3y ago
That’s why it’s important to find a team and coworkers that aren’t so uptight.
- slaymaker1907 3y ago
  With a project that could live on for many years, I’d object to this code, but only because it doesn’t really explain _why_ it’s necessary despite wasting time/memory. The humorous name is great, though. It might be explained in the VCS history, but people aren’t likely to look it up there and I’ve noticed the history tends to get lost for a lot of old projects.
  - eru 3y ago
    Fortunately, git helps you stitch together histories (without changing hashes), if you later recover it: https://git-scm.com/book/en/v2/Git-Tools-Replace https://git-scm.com/book/en/v2/Git-Tools-Replace
  - pizlonator 3y ago
    The name isn’t just funny. It’s also accurate. And it’s quite evidently stood the test of time. In a VM, often the most important kind of documentation is about labeling what is the fast path and what is the slow path. This method name alerts the code reader that they’re looking at one of the slow paths.
- spopejoy 3y ago
  Isn't part of the goal with deliberately weird names like this is to alert callers that HERE BE DRAGONS so, like, don't call this unless you've read _and understood_ the source code?