Skip to content
Permalink
Browse files
@mikesherov says strict is redundant when node is already there
  • Loading branch information
@jaubourg
jaubourg committed Oct 19, 2012
1 parent 9346c0e commit 739ba96318f65d2f2c3c80772519fc0887a8c798
Showing 1 changed file with 0 additions and 1 deletion.
1 .jshintrc
View file
@@ -9,7 +9,6 @@
"node": true,
"quotmark": "double",
"smarttabs": true,
"strict": true,
"trailing": true,
"undef": true,
"unused": true

39 comments on commit 739ba96

@timmywil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm? I don't see how they're related. node is for allowing globals that are present in a node environment. strict is for checking strict mode in function scope.

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/jshint/jshint/blob/0cc64419552b3422ed2d7a600e3047d442a10333/src/stable/jshint.js#L969

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, wrong line: https://github.com/jshint/jshint/blob/0cc64419552b3422ed2d7a600e3047d442a10333/src/stable/jshint.js#L971

@timmywil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, they should document that.

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they certainly should!

@jaubourg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm really not a fan of how jhint handles its flags. Too many implicit dependencies for its own good.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how much of a performance hit we'll be taking for this... strict mode is has significant performance costs that are attributed to the runtime checks that are required.

JFTR...

-1

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @rwldrn on this one. Strict mode is cool and all, but represents a change that we should probably discuss together first before landing. If we did, and I missed that boat, I apologize. If not, we should back this out and talk about it first before re-landing.

@dmethvin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They DID document it, right here! :trollface:

But in this case jshint is just checking against the static use strict requirements, it's not enforcing any runtime checks, right? I like the idea of being use strict clean but not sure I'm ready for a runtime relationship.

@timmywil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwldrn I thought a major point of strict mode was to help interpreters optimize. Is this a situation where it turned out to be a wash?

@timmywil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a side note. I'm +1 on removing for now.

@jaubourg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, what should we remove? jshint checking for strictness or "use strict" (as of now, you cannot have the first without the latest). I'm also as suprised as @timmywil regarding performance impact, I too thought the point was to make it easier to optimize code...

@jaubourg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do I go about benchmarking the strict build to see what the impact actually is? It feel wrong to me to take decisions without metrics. Never used our benchmarking suite so I'll need some help here.

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feel wrong to me to take decisions without metrics.

Exactly. The decision to add it needed to have metrics first 😃

@jaubourg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not like anyone raised those concerns in the PR :P

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I missed it. To be honest, I think we were all a bit loopy by the third day!

@jaubourg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hear you ;)

Anyway, here is my line of reasoning: jQuery in strict mode is good (at least that's what I think). Concerns were raised about performance impact, let's actually establish this performance loss exists before removing. Because everything I read on the interweb (@rwldrn's comment got me really curious) seems to indicate there is strictly no perf impact one way or the other.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mobile, so I can't really dig in and do all of the google-ing for everyone, but think of it like this: if process A has 2 steps and process B is the same algorithm, but takes 4 steps because it has additional runtime checks to evaluate (everytime), which will perform better? Obviously A, because it only takes 2 steps to complete.

@timmywil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwldrn: your theory is good, but I have a feeling it's too simplistic. Is strict mode for javascript engine optimization? Are you saying it helps compile time but not runtime? If so, aren't there more factors to consider during runtime than simply the number of steps (such as inline caching)?. Yes, under the worst of circumstances, a system designed to take 4 steps should take longer than 2 steps, but only if those 2 steps are identical or faster than the corresponding 2 of the 4 steps AND only if those 4 steps can never take shortcuts (perhaps turning 4 steps into 1). Even if strict compliance only helps the interpreter and not execution, there is still the question, "can the additional optimizations of the interpreter outweigh the extra steps taken during runtime?"

As @jaubourg pointed out, this all seems somewhat moot at these low levels anyway since browsers supporting "use strict" are fast enough that the difference is probably negligible.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwldrn: your theory is good, but I have a feeling it's too simplistic. Is strict mode for javascript engine optimization?

No, it's for turning common mistakes into thrown exceptions and locking out access to language features deemed "bad".

Are you saying it helps compile time but not runtime?

Some things are compile time errors and others are runtime.

If so, aren't there more factors to consider during runtime than simply the number of steps (such as inline caching)?.

Sure, but "strict mode" as it's specified, adds more work to the runtime.

Yes, under the worst of circumstances, a system designed to take 4 steps should take longer than 2 steps, but only if those 2 steps are identical or faster than the corresponding 2 of the 4 steps AND only if those 4 steps can never take shortcuts (perhaps turning 4 steps into 1). Even if strict compliance only helps the interpreter and not execution, there is still the question, "can the additional optimizations of the interpreter outweigh the extra steps taken during runtime?"

A few months ago, I wrote a complete use-case analysis of strictmode to help myself understand it, I've made a gist to share: https://gist.github.com/3920242

As @jaubourg pointed out, this all seems somewhat moot at these low levels anyway since browsers supporting "use strict" are fast enough that the difference is probably negligible.

Here's two v8 implementors: https://groups.google.com/forum/?fromgroups=#!topic/v8-users/QVXyJKNShJQ

@timmywil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's for turning common mistakes into thrown exceptions and locking out access to language features deemed "bad".
Some things are compile time errors and others are runtime.

Let me rephrase. Leaving out runtime errors (which would seem to at least add extra conditionals in certain places), any compile time errors that may be determined before execution would theoretically help optimize execution, correct? For instance (and I'm just guessing here), when a variable must be found at runtime, we know it can be found before getting to the global object (unless it is explicitly a variable on the global object) if and only if we are in strict mode, since we already determined that there were no accidental globals at compile time.

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's two v8 implementors: https://groups.google.com/forum/?fromgroups=#!topic/v8-users/QVXyJKNShJQ

No performance benefit !== performance hit.

No, it's for turning common mistakes into thrown exceptions and locking out access to language features deemed "bad".

I assumed that some of those "bad" things were deemed bad because of their performance and optimization implications, no?

The bottom line comes down to this: show me any data proving or suggesting that strict mode is slower and I'll immediately back down on that point. In the meantime, it seems like strict mode may be useful in exposing bugs that may otherwise be hidden. On the other hand, JSHint takes care of most of this stuff for us anyway.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I'm having is that I've been "chiming in" whenever I have a moment as I travel back to Boston, and haven't been able to sit down and produce any worthwhile evidence. Anyway, I'm beginning to not give a shit about it.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also... I'm pretty sure the code is already "strict mode" compliant, without adding the directive.

@mikesherov...

I assumed that some of those "bad" things were deemed bad because of their performance and optimization implications, no?

"No" indeed. You missed something: semantically bad. Most of the features that are blocked are semantic mistakes (of the language), blocked to protect the user code from doing insane things.

@mikesherov
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, anyway, I'm not sure strict mode will like our globalEval function: http://whereswalden.com/2011/01/10/new-es5-strict-mode-support-new-vars-created-by-strict-mode-eval-code-are-local-to-that-code-only/

@jaubourg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikesherov

Hmm, anyway, I'm not sure strict mode will like our globalEval function: http://whereswalden.com/2011/01/10/new-es5-strict-mode-support-new-vars-created-by-strict-mode-eval-code-are-local-to-that-code-only/

The strict mode propagation into eval may be a problem. AFAIK, the only reason we switched to eval rather than script tag injection is that firefox sometimes didn't execute injected script tag with inline code synchronously (it would block execution when other injected, non-async, script tag were pending -- think outbound JSONP). I dunno what the status on this issue is now but we may be able to revert to script tag injection if needs be (would make @jdalton happy actually).

@rwldrn

I know how it is to be on a mobile env ;) Any data you can come up with regarding a potential performance regression is welcome whenever you have time.

Also... I'm pretty sure the code is already "strict mode" compliant, without adding the directive.

Actually there was only one place where we were potentially breaking (but it's just that jhint's static analysis can go so far). However, testing for the code to be strict mode at build time but not being strict at runtime seems to completely defeat the purpose. That's probably why jshint insists on "use strict" being in files it tests for "strictness".

Also, I clearly remember that strict mode was sold on the promise that getting rid of the semantics issues ES4 had would help optimize code at runtime. Maybe it changed along the way (and I'd understand why) but it clearly wasn't just about static analysis (dunno why the last two words got eaten by the intertubes).

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES3* (no ES4, sorry to be pedantic). Ive never seen strict mode sold as an "optimization" (mostly because that's simply not what it's for)

@jaubourg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwldrn Don't worry about being pedantic, we need pedantic people ;)

As for being sold as an optimization:
http://hacks.mozilla.org/2011/01/ecmascript-5-strict-mode-in-firefox-4/

What does strict mode do? First, it eliminates some JavaScript pitfalls that didn’t cause errors by changing them to produce errors. Second, it fixes mistakes that make it difficult for JavaScript engines to perform optimizations: strict mode code can sometimes be made to run faster than identical code that’s not strict mode. Firefox 4 generally hasn’t optimized strict mode yet, but subsequent versions will. Third, it prohibits some syntax likely to be defined in future versions of ECMAScript.

Cannot be any clearer than that and it's not the only place where I saw such claims... but it was in early 2011 and I could understand if this didn't go as planned.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That source is not reputable, as they have published numerous erroneous statements and frequently allow code examples to become out dates. Furthermore, Chris Heilman is not an authority on the subject matter, nor does he back his claims with even the slightest shred of supporting content... But when I produce a point-by-point gist of all strict mode scenarios, I'm probably full of shit right?

Anyone can write a blog and submit it to hacks.mozilla...

@jaubourg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the MDN seems to agree with him:
https://developer.mozilla.org/en-US/docs/JavaScript/Reference/Functions_and_function_scope/Strict_mode

Strict mode makes several changes to normal JavaScript semantics. First, strict mode eliminates some JavaScript pitfalls that didn't cause errors by changing them to produce errors. Second, strict mode fixes mistakes that make it difficult for JavaScript engines to perform optimizations: strict mode code can sometimes be made to run faster than identical code that's not strict mode (Firefox 4 generally hasn't optimized strict mode yet, but subsequent versions will). Third, strict mode prohibits some syntax likely to be defined in future versions of ECMAScript.

The similar wording immediately caught my eye. Apparently (looking at the history of the page), a lot of infos from Chris Heilman's blog post has been ported pretty much verbatim into the MDN article, so maybe it's just the MDN people not being properly informed. If those claims are pure fantasy, then you have to submit edits to the MDN.

As for your gist, as informative and documented as it is, it just lists strict mode restrictions: it gives no indication of potential performance changes one way or the other. Performance impact is probably much more indirect than you seem to believe.

Let's look at the arguments object for instance: not having to expose the call stack through arguments.callee means that the VM doesn't have to adapt its internal call stack structure around it, giving opportunities for optimization. On the other hand, I'd expect enforcing the object is read-only to degrade performances. However, it cannot be worse that the overhead of trying to change the value of a field of a locked object (the former probably being implemented using the latter).

in the end, whatever our guess work, the impact of performance is impossible to evaluate mainly because most of the added logic is in the VM internals, so it's all dependent on internal architecture, native code optimization and native-to-JS bridging. Like I said, very complex. So, I still think we need to benchmark here.

However, from what I could gather in the last few hours, I doubt we'll see any significant performance degradation.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My gist only lists strict mode restrictions because that's all strict mode is... I've been trying to explain this. It's EXTRA instructions that the runtime must carry out.

Look, all I wanted was for everyone to think about the consequences first, before just jumping into something because JSHint says so. I got what I wanted and you all made your decision, so great, all in a days work.

As for MDN, it's a wiki, anyone can write anything on it and I know that Christian works on docs, so he likely copied it himself (that is purely speculation).

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I requested the ability to test for strict mode compatibility without actually using strict mode: jshint/jshint#504

It was apparently closed 2 months ago with no explanation :-(

@curiousdannii
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://groups.google.com/forum/?fromgroups=#!topic/v8-users/QVXyJKNShJQ might be relevant, though I don't know if jQuery ever does that.

http://stackoverflow.com/questions/3145966/is-strict-mode-more-performant/6103493#6103493 suggests that these additional checks will be made regardless of whether or not you use strict mode, but the implementations might do it differently.

@rwaldron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That post was doing well until it invalidated itself by claiming that strict mode is "parse time"

@jdalton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, just to my two cents. I used to have Lo-Dash default to having use strict in its minified code (before making it a build option for Underscore compat). I didn't really see a big negative perf impact from it.

Current engines do have some penalties for strict mode checks, but it's probably micro.
That said, I want to encourage use strict. I wouldn't be surprised to see engines start enticing use by optimizing for strict mode.

@jdalton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the global eval. jQuery's current use window[ "eval" ].call( window, data ) is an indirect eval call so considered global under ES5.

@jdalton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@paulirish Do you know if V8 is optimizing for strict mode atm? I have an inference in Lo-Dash for it because it seems to (if not, maybe it just handles strict mode code less worse than others).

@jdalton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some use strict gotchas I ran into with Lo-Dash for Underscore compat were things like in non-strict mode, writing to a read-only property would not throw an error (it does in strict mode).

@paulirish
Copy link
Member

@paulirish paulirish commented on 739ba96 Oct 21, 2012 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.