SlowDownAndWasteMemory()
github.com
github.com
// 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.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...
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.
Unless it's the name of an HTTP header?
I know that Phillip Hallam-Baker probably first used that spelling in a document
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.
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?
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.Code review can help you prod you to find ways to simplify your code, so that you have fewer paths to consider.
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.
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.
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.
"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.
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...
--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.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.
--i-am-really-stupid - Omits root user privileges check (why would you do that?)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?
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)
Do you mean https://surf.suckless.org ?
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.
https://github.com/WebKit/WebKit/blob/b305f64a67e288b714a84f...
// 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.https://github.com/WebKit/WebKit/blob/ab10a90523e06df54bbb8a...