24 ms·
I mean, I get it, but I think performance is overrated in this particular case; unless it’s a significant and/or very noticeable difference, stick to object lit
by 19ylram49 7y ago
I mean, I get it, but I think performance is overrated in this particular case; unless it’s a significant and/or very noticeable difference, stick to object literals, please. I’d probably fire someone if I started to see `JSON.parse(…)` everywhere in a codebase just for “performance reasons” … remember, code readability and maintainability are just as important (if not more).
- edf13 7y agoNo there’re not
- chuckgreenman 7y agoMost development time is going to be spent on reading code that's already written, so yes, they do matter. With the speeds mentioned it's not going to be appreciable until you hit a massive scale, which, let's face it, most of us aren't working with.
- edf13 7y agoMost dev time for people refactoring code - yes... but not for new projects. And as you say some people do write at scale. > code readability and maintainability are just as important (if not more). This is wrong, that’s all I was saying. Code right and it is readable anyway
- tracker1 7y agoWell, we're talking about injected variables... const injectedValue = JSON.parse("$SERVER_JSON_VALUE.replace("\"","\\\"")"); // vs const injectedValue = $SERVER_JSON_VALUE; generally for a single value in the codebase is emphatically NOT a huge issue... and if it saves 80-120ms or so on the load, that's a significant impact. Not to mention the lower memory overhead while doing so.
- Klathmon 7y agoThey say in the linked article that this should only be used for objects about 10kb and larger. I'd argue that if you have 10kb or larger object literals in your codebase, you are already missing the mark on readability and maintainability in some ways.
- ricardobeat 7y agoWhere you'll usually find this: - exporting data from server to client for initialization - localization data - environment variables (feature maps, configuration etc) - preloading datasets for graphs/tables
- sbr464 7y agoAlso cache initialization scenarios, larger datasets used for common dropdown/select lists like countries w/ ISO codes etc.
- manfredo 7y agoIf it's exclusively going to be used for heavyweight operations like these, it's probably better to benchmark against protobuf decoding. I guess using JSON has a "works out of the box" appeal, and doesn't require defining any protobuf schema. But personally I don't see defining proto files as too prohibitive in terms of development cost.
- kllrnohj 7y ago> benchmark against protobuf decoding Protobuf isn't built into the browser, so it can't bypass the JS parse & execute time. Instead you'd be parsing protobuf's JS, executing it, parsing proto, and producing objects. It'd be worth doing, sure, but it'd almost certainly be the slowest option by far since it's doing way more stuff in JS than either of the other two options and the JS syntax parse is the slow part.
- manfredo 7y agoThese benchmarks indicate better protobuf performance [1]. Compute time these days is often dominated by memory transfer rates. The "slowness" of javascript seems to be offset by there being less data to begin with. Collapsing a 100KB resource down to, 50 or 25KB is usually worth it even if you have to do more operations in javascript. Not to mention end to end load time (which is probably what people are usually trying to optimize for) can be lower by reducing how much data needs to travel over the wire or radio. At the end of the day, who knows if the use case hits edge cases or stresses parts of the implementation that is not optimized for JSON decode or protobuf. Getting meaningful performance data ultimately needs to be experimental, and resists categorical answers about whether X is faster than Y. 1. https://www.npmjs.com/package/protobufjs#performance https://www.npmjs.com/package/protobufjs#performance This article goes into a bit more detail: https://auth0.com/blog/beating-json-performance-with-protobuf/ https://auth0.com/blog/beating-json-performance-with-protobu...
- jackcodes 7y agoI wouldn’t mind having this in my build step, as it’s all minified and unreadable anyway, so what do I care, but I agree with you fully. Not only would you be missing out on readability, none of your linters will catch errors within that string any more and if you use something like prettier, well, god help you. You’re almost guaranteed to introduce more wasted time than you’ll save with this doing it manually.
- lacker 7y agoWell, they are suggesting it for literals that are 10 kB or larger. That means they aren't really talking about code that's in your normal codebase - it's quite rare to have a literal that large. It is more likely this is relevant for backend tools that autogenerate JavaScript code to be sent to a client.
- DrJokepu 7y agoI would fire middle managers for firing individual contributors for trivial, easily correctible issues like that.
- spocklivelong 7y agoWe need managers that mentor and train, instead of firing someone over silly things.
- tzs 7y agoThe JSON.parse approach might also be useful if the same data needs to be used in non-JavaScript code too. You could then use the same string in JSON.parse(...) in your JavaScript, json_decode(...) in your PHP, JSON::Parse's parse_json(...) in your Perl, json.loads(...) in Python, and so on. If you do have constant data that needs to match across multiple programs, it will probably be better in many or even most applications to store the constant data in one place and have everything load it from there at run time, but for those cases where it really is best to hard code the data in each program, doing so as identical JSON strings might reduce mistakes.
- dahart 7y agoIt'd certainly be a good idea to understand exactly what the alternative is when you see JSON.parse() before deciding it's bad or firing anyone, right? There are definitely some legit cases for JSON.parse(). Not to mention that a full round of you setting clear expectations, giving examples of what's recommended and what's not, giving people a chance to learn & grow, and documenting repeat offenses, should all be done before booting someone...? Deep-copying JSON objects using stringify+parse is not just faster, but less problematic and less code than writing a recursive object copy routine.
- flabbergast 7y ago> I’d probably fire someone if I started to see `JSON.parse(…)` everywhere in a codebase just for “performance reasons” … Yep, and I'd fire you for doing that! There are better ways to manage instead of showing off your authority. Oh, and by the way, would some JSON.parse statements for performance be the worst thing in your codebase(s) you guess? I mean, I cannot believe that would be the worse in your codebase. Also, if it really helps to use some JSON.parse for creating big objects for performance reasons, who cares? Instead of firing 'someone' maybe you can add some annotation to it for readability (or if that is below your imaginary level, ask the developer if he/she can add that). Sry, but I hate people that misuse their authority by imposing their subjective opinions.
- willis936 7y agoWould you also fire yourself?
- alexis_fr 7y agoThat’s the compiler/minifier’s role anyway, to use the best construct when appropriate. See Java’s whole “abc”+”ced” vs StringBuilder performance issues. When programmers have to alter readability for performance, it doesn’t necessarily mean they shouldn’t do it, but it means the precompiler is not advanced enough.
- samtheprogram 7y agoI wish could upvote this more than once. Readability is crucial in code. If you have to through and change the JSON that's being parse and it takes a nontrivial amount of time, that's a big setback. Sure, it's 1.7x faster (in v8) to parse JSON, but how long does it take to parse 10kb of an object literal in the first place? Given that these static, large objects are not common place in a codebase, is it worth the tradeoff? The precomiler, such as Babel, could introduce a plugin for this sort of optimization. We only write ASM when it going to significantly change the performance characteristics, and typically when a particular code path is run many, many times throughout an application. If an object literal like this is getting parsed that frequently, there are better ways to optimize so that doesn't need to happen at all anyway. I could see this being very useful in a variety of applications, such as server side rendering. However, its would be best to happen in an optimization phase as you're already bundling at that point.
- untog 7y ago> remember, code readability and maintainability are just as important (if not more). I don't know about that. Prioritising making your own job easier over the experience of all your end users feels like a much more fireable offense to me. In this particular case I'm still a little wary of it because it feels like it's optimising for a current implementation with no idea what the future performance implications might be (or current implications in non V8 engines?) but this trend of prioritising developer experience over everything feels like a very bad one to me. It's the same reason given to justify making every web site a React app with no thought toward the extra JS payload you're sending when it's not needed.
- partialrecall 7y agoUsers like code with fewer bugs and rapid response time for new feature requests, right? If you start firing people for taking the time to write readable and maintainable code, you'll be doing a greater disservice to the users than those developers were.
- yiyus 7y agoThe graph in the article includes results for other engines too, not only V8.
- wnewman 7y agoThis hack is supposed to be for huge data: 10kb or more, thus comfortably more than a page. If the >10kb wall o' code was wrapped in a parse-as-JSON-at-runtime function call which was was preceded by a three-line comment describing a quick and dirty benchmark showing that it saves a useful number of milliseconds on page load in a fairly typical use case, and if the web resource was intended to be loaded many millions of times, I would nod and approve when reviewing the code. The way the original objector writes, it sounds as though nothing would suffice to justify this hack, and certainly not a mere benchmark and 3 lines of comments preceding it. That attitude seems like unreasonable blinkered zealotry, or some other kind of tunnel vision, e.g. someone who has just never thought seriously about the appropriate tradeoffs in maintaining a web resource which gets loaded millions of times a month.
- qtplatypus 7y ago
- eyelidlessness 7y agoI totally agree that inlining `JSON.parse` of string literals in source is a bad idea and I would reject it in a code review except under the most extreme circumstances (and even then try to identify a better solution). On the other hand, knowing the performance characteristics, this is something that compilers could do as an optimization. Who knows if that's worth the effort, but this kind of research is part of determining that.
- SirensOfTitan 7y ago> I'd probably fire someone if I started to see `JSON.parse(...)` I've had the privilege of working in organizations that consider mistakes to be the cornerstone of resilient systems. Because of that, comments like this scare me, even when intentionally hyperbolic. More so, if the product works well and is being maintained easily, why would you micromanage like that? Sounds like a minor conversation only worth having if the technical decision is having a real impact. Thomas J. Watson: > Recently, I was asked if I was going to fire an employee who made a mistake that cost the company $600,000. No, I replied, I just spent $600,000 training him. Why would I want somebody to hire his experience?
- dkersten 7y agoYou probably wouldn't want to work for somebody who fired people so easily anyway. This is one reason why I find it stupid when people defend companies or are super loyal to their employers: companies don't care about you and especially companies that fire on a whim without concern that they're fucking with somebodies life. Best to work somewhere that treats you like a human instead of as a cog.
- filoleg 7y agoEh, depends on the reasons for firing people easily. Firing easily for honest errors is moronic, fully agreed, especially if the person is learning from them. My code changes caused more than one sev0 before, but never was I personally blamed for them, as it was always some bigger underlying system issue that wouldn't have allowed me to make those mistakes, if the systems were more robust (and I was a little bit more wise and not pushed "seemingly safe" changes outside of business hours). I learned a lot from those mistakes. Firing easily for a long history of non-improvement and not meshing well with the team (underperforming, causing a lack of cohesion within the team, etc.) is good for the team, but in principle it is similar to the "good king" kind of approach, so it all relies on the "king" having a straight head. P.S. My last paragraph does not imply "culture fit" or any superficial stuff like that as a good reason for firing, I meant more fundamental sort of issues, like refusing to listen to people, never even attempting to improve (given you have some hiccups, just like most of us), etc.
- thoughtpalette 7y agoBelieve you can use "Interceptors" or the Adapter pattern on the Front-end to easily use JSON.parse once for all your http calls instead of littering it throughout the code base.
- 5trokerac3 7y agoFirst paragraph... > This knowledge can be applied to improve start-up performance for web apps that ship large JSON-like configuration object literals Third paragraph... > A good rule of thumb is to apply this technique for objects of 10 kB or larger — but as always with performance advice, measure the actual impact before making any changes. I'd fire people who don't RTFM
- deleted 7y ago[deleted]
- deleted 7y ago[deleted]
- tracker1 7y agoFor the main two apps I work on, there's some configurations that are different between different client deployments, this includes i18n strings, configuration settings/options, theme options and a couple of images (base64 encoded) for theming. Switching to JSON.parse was a pretty significant impact, from about over 200ms to under 100ms for my specific use case (IIRC). Memory usage was also reduced. I don't remember the specific numbers... it was an easy change in the server handler for the base.js file that injects a __BASE__ variable. var clientConfig = JSON.Stringify(base.Env.Settings.ToClient(null)).Replace("\"", "\\\""); // NOTE: JSON.parse is faster than direct JS object injection. ClientBase = $"{clientTest}\nwindow.__BASE__ = JSON.parse(\"{clientConfig}\")"; ... return Content($"{ClientBase}\n__BASE__.acceptLanguage=\"{lang}\";", "application/javascript"); The top part is actually a static variable that gets reused for each request, the bottom is the response with the request language being set for localization in the browser app.
- beatgammit 7y agoThey mentioned that it should only be used for very large objects (say, 10k), so if you're seeing ~10k, hard-coded objects throughout your code, you should probably fire someone. If it's in just a few places, there should be a comment describing it (e.g. "large object constructed from DB query, use JSON to make page load faster").
- geddy 7y ago> I’d probably fire someone if I started to see `JSON.parse(…)` Guys - I think he was being hyperbolic. Ya know, like everyone does on the Internet. If he had said "if I had to look at JSON.parse(...) lines constantly, I'd jump off a building!" I doubt you all would be calling 911 over an attempted suicide. Seriously, chill.
- iamleppert 7y agoWhy do you care? It’s syntax and can be automated via build tools so you need not hurt your eyes with syntax that you consider to be unpleasant. Which that’s the crux of the issue here, your opinion.
- quickthrower2 7y agoIf I used this one weird trick, I'd want it to be compile time checked. I'd stick that JSON in a separate file, get typescript to compile it "just to check it's OK" then get the compiled code and include it as a string using something like https://webpack.js.org/loaders/raw-loader/ https://webpack.js.org/loaders/raw-loader/, I guess (not used it before). There might be a leaner way to do this (maybe the whole thing can be done as a webpack loader in one step), but something like this.
- AgentOrange1234 7y agoTFA says this is could make sense for objects over 10kb. They clearly aren’t advocating doing it everywhere in a code base.