// We play this game because we want this to be callable even from places that
// don't have access to CallFrame\* or the VM, and we only allocate so little
// memory here that it's not necessary to trigger a GC - just accounting what
// we have done is good enough. The sort of bizarre exception to the "allocating
// little memory" is when we transfer a backing buffer into the C heap; this
// will temporarily get counted towards heap footprint (incorrectly, in the case
// of adopting an oversize typed array) but we don't GC here anyway. That's
// almost certainly fine. The worst case is if you created a ton of fast typed
// arrays, and did nothing but caused all of them to slow down and waste memory.
// In that case, your memory footprint will double before the GC realizes what's
// up. But if you do *anything* to trigger a GC watermark check, it will know
// that you *had* done those allocations and it will GC appropriately.
Kind of interesting, really. The method is clearly somewhat sarcastically named but usefully so: they know what they're doing is not optimal but still do it anyway for the reasons outlined. It's not like this is laziness or nefariousness.
pizlonator 774 days ago [-]
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 774 days ago [-]
Indeed you did, 10 years ago. This is the (mega!) commit:
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 774 days 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.
phatskat 774 days 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.
JohnBooty 773 days ago [-]
how do you effectively manage them in the review process
The overriding principle is "no surprises at final review time."
For a big impactful change discuss the work in progress beforehand (specifically: architecture stuff, database changes, impact this will have on other code) with them as much as possible before final review time. There's no other sane way to do it.
Make sure you're all in agreement on the approach you're taking or, at least, make sure they're aware of it. Also a good way to (hopefully) make sure you're not going to conflict with somebody else's work!
Dumping a big commit full of surprises on reviewers is AT BEST poor practice and is at worst kind of shady and a sign you don't want to collab, don't respect your coworkers and/or or are hiding something.
phatskat 773 days ago [-]
I definitely prefer in avoiding surprises. In this case the review was upgrading and migrating from Vue 2 to Vue 3 and, while the rest of the team was in the loop and aware of coming changes, the change set itself was massive. I definitely would do it differently the next time around, and this is an edge case. I will say that that position saw several “larger than any reasonable person should submit” change sets, glad it’s behind me.
JohnBooty 773 days ago [-]
Sounds like you did well! Congrats on the big upgrade - not easy.
pizlonator 774 days ago [-]
It’s simple. Find a reviewer who isn’t that thorough.
throwaway2037 773 days ago [-]
I call this "review shopping". If someone is pushing back on a change, find a weaker link.
avg_dev 773 days ago [-]
I laughed
leoh 773 days ago [-]
Just don't tell Oliver
pizlonator 773 days ago [-]
Oliver is the best reviewer ever
8n4vidtmkvmk 774 days 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.
pdpi 774 days ago [-]
In this particular case, we're talking some tricky code around array handling in the JS implementation of a major browser. It's pretty much the archetypical location for a VM escape bug. That's most definitely not something you want to be cavalier about.
In general, the large, sprawling diffs we're talking about here, the sort that actually justify commits of this size, are almost always going to also be the sort that justify more closer scrutiny at review time.
pizlonator 774 days ago [-]
I doubt that a thorough review is going to find security bugs in a change like this.
Good tests, a great fuzzer, and a highly motivated security research community - that’s how you find the great security bugs!
olliej 774 days ago [-]
No. In any actually complex piece of code even adding significant amounts testing over new code is not going to cover every possible code path.
It's also immensely difficult to write tests to find errors in code you're writing - most patches I see with "extensive tests" test all the correct code paths, and _maybe_ a few error cases. It's a very easy trap to get yourself into.
The purpose of review is not to catch obvious issues - though it hopefully will - but the subtle ones.
troupo 773 days ago [-]
> and _maybe_ a few error cases.
Also it's often unit tests only, with no hint of any integration tests. I'd settle for a tests that check integration of at least two units, but alas.
hgsgm 774 days ago [-]
Purpose of review is to ask questions to help you write more tests to catch more bugs.
pizlonator 774 days ago [-]
I’ve had both thorough and cursory reviews of changes like this.
It makes no difference for finding actual bugs.
Reviews are great for spreading knowledge about the change and making sure the style is dialed in. Reviews aren’t great for ensuring test coverage or finding security bugs.
eru 773 days ago [-]
Test coverage should be ensured by your coverage tools in the CI. (At least basic test coverage.)
You are right that reviews aren't necessarily there to catch bugs. Reviews are there to tell you to write code that's simple and understandable. If your coworkers can't understand what's going on, how could you expect anyone else to do so in a year or so?
olliej 773 days ago [-]
Coverage tools only tell you have tests that hit each line, but most serious bugs aren't the "you didn't hit one line", they're the result of specific interactions between multiple paths. e.g. (note this is a super simple and trivial example)
int size = 0;
char *buffer;
if (a) {
buffer = new char[1];
size = 1;
} else if (b) {
buffer = new char[2];
size = 2;
}
...
if (b) {
buffer[1]++;
}
...
Coverage tests will happily show that every line is hit, without tests necessarily hitting the buffer overflow if an and b are both true.
eru 773 days ago [-]
I agree. Hence my parenthetical remark about '(At least basic test coverage.)'
Code review can help you prod you to find ways to simplify your code, so that you have fewer paths to consider.
londons_explore 774 days 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?').
You might want to check how that word was spelled in the comment you're replying to :)
pacaro 773 days ago [-]
Woosh. Also Henrik is Danish, he can claim that English isn't his first language
js2 772 days ago [-]
Henrik?
pacaro 762 days ago [-]
I worked for a while with Henrik Frystyk Nielsen, who worked on the early HTTP spec and httpd. We would give him a hard time for this — among other things.
I know that Phillip Hallam-Baker probably first used that spelling in a document
773 days ago [-]
rezonant 772 days ago [-]
Well done :-D
xvector 774 days ago [-]
This kind of mentality is absurdly inappropriate when working on something as security sensitive as a browser's JS engine.
amne 773 days ago [-]
This is code running on client machines which you have no control over. Once it's out there good luck getting everyone to upgrade to your latest version because "hey, we didn't want this one to behave like that. oops". Especially when you're google and don't want certain freedoms in your software.
8n4vidtmkvmk 773 days ago [-]
That's fair. I mostly work on servers and web FE which is also easy to redeploy.
jvanderbot 774 days ago [-]
You make sure that more than one person is aware of the changes and design, and have all people frequently discuss the progress before the mega patch hits PR.
Then the mega patch PR is " one last look ", and not thousands of changes storming the gates of your sanity.
hgsgm 774 days ago [-]
How do you know if the "discussion" made it into the code?
eru 773 days ago [-]
You can formalise that discussion by breaking the patch into smaller commits.
lamontcg 773 days ago [-]
Yeah its best to go over something like this is a meeting/zoom or a series of meetings. And most likely not every single line will be gone over, at some point the engineer writing this code is likely pretty senior and shouldn't need that many eyes on their boilerplate, and its the nasty stuff that really needs talking about and focusing on.
IshKebab 774 days ago [-]
You just don't review it as well. It's not the end of the world.
hgsgm 774 days ago [-]
It's just your bank accounts, your crypto, your Gmail with all your passwords to everything....
IshKebab 773 days ago [-]
Code review doesn't magically catch all bugs even with tiny PRs. At best it's a basic sanity check. In fact I'd say it's more of a design sanity check than a bug catching one.
To prevent bugs you need tests, fuzzing, static analysis, static & strong type checking, etc.
bpye 774 days 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 774 days 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.
kccqzy 774 days 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 774 days ago [-]
I don't want a personal review, I want a code reviewer.
"Trusting" someone means normalization of deviance, where bugs slip in.
eru 773 days 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 774 days ago [-]
This is classic HN.
"Look at this crazy, obscure code everybody!"
*ORIGINAL DEVELOPER APPEARS*
astrange 774 days 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 774 days 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 774 days 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 773 days 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 773 days ago [-]
That’s why it’s important to find a team and coworkers that aren’t so uptight.
slaymaker1907 773 days 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.
pizlonator 773 days 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.
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?
I can't find the reference right now, but IIRC the Glasgow Haskell Compiler had a function called "unsafeDirectIo" which people were using too flippantly. So they renamed it to "evilUnholyDirectIo"
tome 773 days ago [-]
Maybe you're thinking of `unsafePerformIO` (which still exists) and `accursedUnutterablePerformIO`.
Big fan of these “sarcastically named” functions, there are a lot of good reasons to use them (mainly, every time you go to use it you’ll think about whether it’s really needed) and not many reasons to not use it (naming it “fastConvert” doesn’t make it any faster than naming it “slowConvert”).
kstrauser 774 days ago [-]
Some of our internal tools have command line options along the lines of:
--yolo Yes, I really want to overwrite the production database and get us all fired.
where "--yolo" is our standard flag for such things. When I find myself about to type that, I go back and double, triple, quadruple check that I really intend to do the thing that I'm asking it to do.
adhesive_wombat 774 days ago [-]
Shades of hdparm's --please-destroy-my-drive
nurple 774 days ago [-]
Love it! Ceph has --yes-i-really-mean-it for dangerous fs changes, and --yes-i-really-really-mean-it for pool rm. The annoying-to-type property is pretty effective.
snet0 774 days ago [-]
This kind of thing is not so unpopular in some kinds of user interfaces. I think GitHub has a repo deletion confirmation that asks you to type the name of the repo. It's not quite the same as flag naming, but it's a big flashing light signalling to you that you're doing something important.
nurple 771 days ago [-]
I really like that UX! Even Google dropped the ball there, I accidentally deleted our production gke cluster a handful of years back, through the bloodshot eyes of a forced all-nighter, because their deletion UX was two button clicks (may be different now).
scarface_74 773 days ago [-]
At my company I have repositories that are meant to be open source and repositories that are proprietary. We have an internal tool that ask you to type a sentence the first time you push to open source for confirmation.
But to make sure you actually type it in instead. The sentence you’re instructed to type in is lowercase. But the instructions tell you to type it in as uppercase.
throwing_away 773 days ago [-]
Hyprland has:
--i-am-really-stupid - Omits root user privileges check (why would you do that?)
Seen a few “_do_not_use_or_you_will_be_fired()” functions before too.
kstrauser 774 days ago [-]
I applaud that one, too.
vidarh 774 days ago [-]
MySQL of course has (had? Not used it for an eternity) the --i-am-a-dummy flag, which felt like it should be reversed (it prevents update/delete without an explicit where clause, but would be more appropriate if you had to use it to allow them).
8n4vidtmkvmk 774 days ago [-]
I'd assume fastConvert is less accurate. If it was both fast and perfect it'd just be 'convert'
forty 774 days ago [-]
Fast doesn't always mean good. For example in cryptography a fonction named fastCompare could be a non constant time comparison which is faster but not safe for cryptography :D (though you'd probably want to name it unsafeButFastCompare I guess ^^)
BoppreH 774 days ago [-]
Or it only accepts certain inputs, or has an expensive first invocation but amortized cost, or is not bug-compatible with the old `convert`, or it requires a large cache, or it's optimized for parallelism, or uses instructions only available in newer CPUs, or it was a prototype that stuck around, or...
"When you do uint8array.buffer in JSC the first time, it calls slowDownAndWasteMemory()" -- links to the github
The tweet has ~60k views, and 24 retweets.
olliej 774 days ago [-]
alas that's not accessible anymore
vGPU 774 days ago [-]
To try to prevent others from joining this argument: twitter is currently not allowing you to read tweets without being logged in. Hence, it can be both available and not available.
olliej 774 days ago [-]
If it cannot be read without logging in, then it is inaccessible. That's a choice twitter gets to make, but to claim tweets are accessible because some people can read them is no different from posting links to a private facebook group and saying "you just need to be a member of that group".
That said, I was saying it is inaccessible because I am getting error messages trying to load it.
dylan604 774 days ago [-]
>Hence, it can be both available and not available.
So Musk has given us the real life Schrödinger's cat?
DANmode 774 days ago [-]
If I never auth again, Twitter doesn't exist?
topaz0 774 days ago [-]
Last I checked a tweet isn't a real life cat any more than an electron is.
lopkeny12ko 774 days ago [-]
[flagged]
tomjakubowski 774 days ago [-]
almost certainly an innocent mistake, not "lying"
Percent1361 774 days ago [-]
[flagged]
dang 773 days ago [-]
That's far from true and simply indicates the direction (and intensity) of your own political passions. This is one of the most reliable phenomena on HN: everyone thinks their team is the one being repressed. People with opposite passions think the site is overrun by your team and that the mods are in your pocket.
For what it's worth, I can't access it either, so there are probably many of us.
olliej 774 days ago [-]
Yeah, but based on his musk comment I'm going to assume anything we say isn't relevant.
He's accusing people of lying because they hate musk, when they're saying content is not accessible on a site that is currently documented to be suffering basic response problems. It's the kind of reactive BS you get from idiots who aren't interested in facts or reality so shrug.
Faceted4843 774 days ago [-]
>It's the kind of reactive BS you get from idiots who aren't interested in facts or reality so shrug.
Lying because you hate Musk is the reactive BS from an idiot who isn't interested in facts or reality.
olliej 774 days ago [-]
"Something went wrong. Try reloading."
> Y'all's hatred of Musk is driving you to lie.
My god, my hatred for Musk has caused twitter to go down. Wow, I didn't know I had such power. JFC.
One of those irrational times of my year is when I inevitably hit something that really deserves a good descriptor but there just isn’t a good word or two for it. So I end up with one of these monsters.
It’s not a real problem, but it’s a moment that doesn’t bring me joy.
eyelidlessness 773 days ago [-]
I personally love these kinds of names. On a long enough timeline, using them either:
- prompts someone to tell you the actual name for the thing, and now you know a name for something
- calls out poorly considered dependencies/design
- leads to naming a thing that deserves a name
- calls out complexity which is inherent either to the domain or other aspects of the underlying architecture
Every one of those is a positive outcome, and a less verbose name will never* be a better tradeoff.
* There are many apparent real-world exceptions, but they always fall under the “naming a thing that deserves a name” category, even if the resulting name is less clear than the verbose one. And they’re almost always directly involved in core abstractions (i, acc, cons) or core domain concepts.
Edited to make this less mistakenly ignorant of lisp.
nikanj 773 days ago [-]
We used to make fun of Java / Spring for names like this, but it turns out it was just being used for absolutely gigantic systems before other languages.
(Yes I know your great uncle Bobert coded 644 million lines of Cobol for a massive app where all function names were less than 4 characters. I'm talking general trends here)
jakeogh 774 days ago [-]
I like https://surf.suckless.org (which uses WebKit) but it seems like every time I use it and leave a window open for awhile, that process (WebkitWebProcess) will end up pegging a core for no obvious reason. It's not site dependent, and I think it's when JS is enabled.
pizlonator 774 days ago [-]
100% probability that the behavior you observe has 0% to do with this function.
jakeogh 774 days ago [-]
Oh I agree. I suppose I should read https://www.webkit.org/debugging-webkit/.
It's a problem I cant trivially reproduce on command, so (reading your comment above) I wonder how you would approach it.
londons_explore 774 days ago [-]
I would just run "sudo perf record" to do a systemwide performance trace, followed by "sudo perf report" and you'll find the function name where it's spending all the CPU cycles.
Another option is to just "sudo gdb -p 12345" to attach to the process, then "thread apply all bt" to get stack traces of each thread, and find whichever isn't sat in "wait". That has the benefit you can inspect local variables in every frame etc (for opensource stuff, symbols are usually available automatically via debuginfod).
Chromium/blink has much better built in performance tracing things if you can reproduce there - see chrome://tracing/ for tracing internal browser components, or the Performance tab in devtools if you think it is javascript gobbling all the CPU time.
anthk 774 days ago [-]
All webkitgtk4 browsers run the same, from surf to luakit to vimb to midori.
Some of the identifier names are kind of casual here. I was amused by the class called DeferGCForAWhile. I'm guessing just from reading that it's a typical RAII pattern so the "while" that will be deferred may be the scope of the object, i.e. until its destructor is called.
Jarred 774 days ago [-]
You might also like JSGlobalObject::haveABadTime()
// Consider the following objects and prototype chains:
// O (of global G1) -> A (of global G1)
// B (of global G2) where G2 has a bad time
//
// If we set B as the prototype of A, G1 will need to have a bad time.
// See comments in Structure::mayInterceptIndexedAccesses() for why.
//
// Now, consider the following objects and prototype chains:
// O1 (of global G1) -> A1 (of global G1) -> B1 (of global G2)
// O2 (of global G2) -> A2 (of global G2)
// B2 (of global G3) where G3 has a bad time.
//
// G1 and G2 does not have a bad time, but G3 already has a bad time.
// If we set B2 as the prototype of A2, then G2 needs to have a bad time.
// Note that by induction, G1 also now needs to have a bad time because of
// O1 -> A1 -> B1.
//
// We describe this as global G1 being affected by global G2, and G2 by G3.
// Similarly, we say that G1 is dependent on G2, and G2 on G3.
// Hence, when G3 has a bad time, we need to ensure that all globals that
// are transitively dependent on it also have a bad time (G2 and G1 in this
// example).
//
// Apart from clearing the VM structure cache above, there are 2 more things
// that we have to do when globals have a bad time:
// 1. For each affected global:
// a. Fire its HaveABadTime watchpoint.
// b. Convert all of its array structures to SlowPutArrayStorage.
// 2. Make sure that all affected objects switch to the slow kind of
// indexed storage. An object is considered to be affected if it has
// indexed storage and has a prototype object which may have indexed
// accessors. If the prototype object belongs to a global having a bad
// time, then the prototype object is considered to possibly have indexed
// accessors. See comments in Structure::mayInterceptIndexedAccesses()
// for details.
//
// Note: step 1 must be completed before step 2 because step 2 relies on
// the HaveABadTime watchpoint having already been fired on all affected
// globals.
//
// In the common case, only this global will start having a bad time here,
// and no other globals are affected by it. So, we first proceed on this assumption
// with a simpler ObjectsWithBrokenIndexingFinder scan to find heap objects
// affected by this global that need to be converted to SlowPutArrayStorage.
// We'll also have the finder check for the presence of other global objects
// depending on this one.
//
// If we do discover other globals depending on this one, we'll abort this
// first ObjectsWithBrokenIndexingFinder scan because it will be insufficient
// to find all affected objects that need to be converted to SlowPutArrayStorage.
// It also does not make dependent globals have a bad time. Instead, we'll
// take a more comprehensive approach of first creating a dependency graph
// between globals, and then using that graph to determine all affected
// globals and objects. With that, we can make all affected globals have a
// bad time, and convert all affected objects to SlowPutArrayStorage.
rurban 773 days ago [-]
Normal people would call that pessimation(). There are some kind of languages, like perl, which brings out the people with funny names.
kazinator 774 days ago [-]
Love that printInternal method. If the caller wants to map the enum to the string, make them know about a PrintStream that they have to instantiate.
(And possibly write their own derivation, if there isn't one that captures output into a string.)
nektro 774 days ago [-]
relevant code on line 269
traes 774 days ago [-]
Wouldn't it make more sense to directly link to the line in the title?
Ah I was referring to the github specific terminology when sharing a link to a line inside a code file.
ryanisnan 774 days ago [-]
Thanks! I referred to them as anchors.
wiml 774 days ago [-]
I think pedantically anchors are the markers in the document, and the URL fragments refer to the anchors to tell the browser to scroll there. If I'm wrong, someone will be along to correct me...
dang 774 days ago [-]
Ok, I've put it (back?) in there.
Red_Tarsius 773 days ago [-]
Hi dang. Sorry to ask you here but I wasn't sure how to get in touch with the staff. Have you had the chance to review the email I sent to hn@ycombinator.com? I know you're very busy so I don't mean to rush you. I just want to make sure the spam filter didn't get it first hahaha
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.
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...
For a big impactful change discuss the work in progress beforehand (specifically: architecture stuff, database changes, impact this will have on other code) with them as much as possible before final review time. There's no other sane way to do it.
Make sure you're all in agreement on the approach you're taking or, at least, make sure they're aware of it. Also a good way to (hopefully) make sure you're not going to conflict with somebody else's work!
Dumping a big commit full of surprises on reviewers is AT BEST poor practice and is at worst kind of shady and a sign you don't want to collab, don't respect your coworkers and/or or are hiding something.
In general, the large, sprawling diffs we're talking about here, the sort that actually justify commits of this size, are almost always going to also be the sort that justify more closer scrutiny at review time.
Good tests, a great fuzzer, and a highly motivated security research community - that’s how you find the great security bugs!
It's also immensely difficult to write tests to find errors in code you're writing - most patches I see with "extensive tests" test all the correct code paths, and _maybe_ a few error cases. It's a very easy trap to get yourself into.
The purpose of review is not to catch obvious issues - though it hopefully will - but the subtle ones.
Also it's often unit tests only, with no hint of any integration tests. I'd settle for a tests that check integration of at least two units, but alas.
It makes no difference for finding actual bugs.
Reviews are great for spreading knowledge about the change and making sure the style is dialed in. Reviews aren’t great for ensuring test coverage or finding security bugs.
You are right that reviews aren't necessarily there to catch bugs. Reviews are there to tell you to write code that's simple and understandable. If your coworkers can't understand what's going on, how could you expect anyone else to do so in a year or so?
Code review can help you prod you to find ways to simplify your code, so that you have fewer paths to consider.
Unless it's the name of an HTTP header?
I know that Phillip Hallam-Baker probably first used that spelling in a document
Then the mega patch PR is " one last look ", and not thousands of changes storming the gates of your sanity.
To prevent bugs you need tests, fuzzing, static analysis, static & strong type checking, etc.
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.
"Trusting" someone means normalization of deviance, where bugs slip in.
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.
"Look at this crazy, obscure code everybody!"
*ORIGINAL DEVELOPER APPEARS*
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.
I like the name, but can easily imagine somebody preferring a more serious alternative.
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.
https://github.com/WebKit/WebKit/blob/ab10a90523e06df54bbb8a...
Thanks for your contributions to Webkit :)
https://android.googlesource.com/platform/frameworks/base/+/...
https://hackage.haskell.org/package/base-4.18.0.0/docs/Syste...
https://hackage.haskell.org/package/bytestring-0.10.8.1/docs...
But to make sure you actually type it in instead. The sentence you’re instructed to type in is lowercase. But the instructions tell you to type it in as uppercase.
Quote for people without account or rate limited:
"When you do uint8array.buffer in JSC the first time, it calls slowDownAndWasteMemory()" -- links to the github
The tweet has ~60k views, and 24 retweets.
That said, I was saying it is inaccessible because I am getting error messages trying to load it.
So Musk has given us the real life Schrödinger's cat?
https://news.ycombinator.com/item?id=26148870 has an amusing list of examples. I suppose I should make an updated list sometime.
And even then, I've never had a Twitter account, so not sure I'd be able to see it now anyways.
Sure seems inaccessible to me.
"Bean counters said I couldn't fire a man for being in a wheelchair. Did it anyway. Ramps are expensive."
https://i.imgur.com/44KNIlg.jpg
He's accusing people of lying because they hate musk, when they're saying content is not accessible on a site that is currently documented to be suffering basic response problems. It's the kind of reactive BS you get from idiots who aren't interested in facts or reality so shrug.
Lying because you hate Musk is the reactive BS from an idiot who isn't interested in facts or reality.
> Y'all's hatred of Musk is driving you to lie.
My god, my hatred for Musk has caused twitter to go down. Wow, I didn't know I had such power. JFC.
One of those irrational times of my year is when I inevitably hit something that really deserves a good descriptor but there just isn’t a good word or two for it. So I end up with one of these monsters.
It’s not a real problem, but it’s a moment that doesn’t bring me joy.
- prompts someone to tell you the actual name for the thing, and now you know a name for something
- calls out poorly considered dependencies/design
- leads to naming a thing that deserves a name
- calls out complexity which is inherent either to the domain or other aspects of the underlying architecture
Every one of those is a positive outcome, and a less verbose name will never* be a better tradeoff.
* There are many apparent real-world exceptions, but they always fall under the “naming a thing that deserves a name” category, even if the resulting name is less clear than the verbose one. And they’re almost always directly involved in core abstractions (i, acc, cons) or core domain concepts.
Edited to make this less mistakenly ignorant of lisp.
(Yes I know your great uncle Bobert coded 644 million lines of Cobol for a massive app where all function names were less than 4 characters. I'm talking general trends here)
Another option is to just "sudo gdb -p 12345" to attach to the process, then "thread apply all bt" to get stack traces of each thread, and find whichever isn't sat in "wait". That has the benefit you can inspect local variables in every frame etc (for opensource stuff, symbols are usually available automatically via debuginfod).
Chromium/blink has much better built in performance tracing things if you can reproduce there - see chrome://tracing/ for tracing internal browser components, or the Performance tab in devtools if you think it is javascript gobbling all the CPU time.
Do you mean https://surf.suckless.org ?
https://github.com/WebKit/WebKit/blob/b305f64a67e288b714a84f...
https://github.com/WebKit/WebKit/blob/ab10a90523e06df54bbb8a...