-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Buffer() without new keyword? #8
Comments
Where do you find it in the source? I don't recall a single usage of it, so On Wed, Sep 7, 2016, 19:17 crazyhuggins notifications@github.com wrote:
|
It's not in your source code, but you wrote this "This code was initially derived from node-deep-equal by James Halliday", so I just noticed you wasn't supporting it? And this features now have a deprecation warning. |
It's derived from it initially (I wasn't about to take credit), but I've On Wed, Sep 7, 2016, 19:52 crazyhuggins notifications@github.com wrote:
|
https://github.com/teppeis/deeq did you see this? With strict key order are checked for Set and Map. And the new prototypes for map() will work? |
@crazyhuggins I've explicitly coded the algorithm to be independent of key order for all types, simply because relying on key order for traditionally unordered collections is, in my experience, rather bug-prone, and is rarely necessary for the work at hand. (I've only used key order a few times, and all but one of those were later refactored away to something much cleaner.) You may be surprised to find that it actually took extra work to ensure And I do invite you to take a look at the implementation itself. To maybe answer a few more potential questions, here is what it supports:
I made sure to cover a lot of bases. If I'm going to write a deep equality library for testing, I'm going to prefer completeness over ease of implementation. |
Between that and the various optimizations [1] (to keep the running times sane), it's well over 500 lines of code (vs the 120 or so for [1]: Things like checking indices first, avoiding runtime-generated closures (since the code is highly polymorphic), limiting polymorphism where possible, etc. |
I understand it now :) Actually I talked to a friend of mine too yesterday that is an expert on such things, But he noticed there was some obvious bottle necks in your algo? I'm not sure what I mean with that, but he wrote a simple algo himself to point out the differences and where the bottlenecks are. He doesn't have a Github, or maybe he created one yesterday. Not sure :) Yours: NaN x 37,520,025 ops/sec ±0.57% (92 runs sampled)
string literal x 39,753,005 ops/sec ±0.57% (93 runs sampled)
array literal x 309,051 ops/sec ±0.53% (92 runs sampled)
boolean literal x 39,622,088 ops/sec ±0.49% (91 runs sampled)
object literal x 1,172,920 ops/sec ±0.59% (88 runs sampled)
object from null x 1,589,217 ops/sec ±0.75% (89 runs sampled)
regex literal x 1,329,053 ops/sec ±0.82% (91 runs sampled)
number literal x 42,213,429 ops/sec ±0.55% (88 runs sampled)
null x 39,036,191 ops/sec ±0.85% (93 runs sampled)
undefined x 39,348,740 ops/sec ±0.67% (91 runs sampled)
date x 1,411,588 ops/sec ±1.14% (88 runs sampled)
regex constructor x 1,004,130 ops/sec ±0.68% (90 runs sampled)
string constructor x 773,232 ops/sec ±0.86% (91 runs sampled)
string literal (differing) x 29,061,520 ops/sec ±0.43% (91 runs sampled)
array literal (differing) x 2,057,313 ops/sec ±0.82% (89 runs sampled)
boolean literal (differing) x 32,319,124 ops/sec ±0.57% (94 runs sampled)
object literal (differing) x 989,009 ops/sec ±1.02% (87 runs sampled)
regex literal (differing) x 10,122,073 ops/sec ±1.12% (90 runs sampled)
number literal (differing) :
null & undefined x 31,707,553 ops/sec ±0.60% (90 runs sampled)
date (differing) x 8,445,773 ops/sec ±0.59% (92 runs sampled)
error :
regex ctor (differing) x 9,930,519 ops/sec ±0.47% (91 runs sampled)
string ctor (differing) :
weakmap :
weakset :
promise : His (quick dirty work he said): NaN x 39,799,909 ops/sec ±0.16% (91 runs sampled)
string literal x 43,785,121 ops/sec ±0.51% (88 runs sampled)
array literal x 3,854,342 ops/sec ±0.58% (90 runs sampled)
boolean literal x 40,839,804 ops/sec ±0.49% (93 runs sampled)
object literal x 2,317,986 ops/sec ±0.57% (90 runs sampled)
object from null x 3,390,177 ops/sec ±0.64% (92 runs sampled)
regex literal x 15,403,105 ops/sec ±0.78% (90 runs sampled)
number literal x 45,846,439 ops/sec ±0.85% (88 runs sampled)
null x 42,618,306 ops/sec ±0.60% (88 runs sampled)
undefined x 42,727,581 ops/sec ±0.53% (92 runs sampled)
date x 23,136,702 ops/sec ±0.60% (89 runs sampled)
regex constructor x 14,990,824 ops/sec ±0.59% (90 runs sampled)
string constructor :
string literal (differing) x 34,924,710 ops/sec ±0.54% (90 runs sampled)
array literal (differing) x 4,775,895 ops/sec ±0.61% (88 runs sampled)
boolean literal (differing) x 37,511,439 ops/sec ±0.53% (91 runs sampled)
object literal (differing) x 2,199,399 ops/sec ±0.68% (90 runs sampled)
regex literal (differing) x 23,134,269 ops/sec ±1.03% (89 runs sampled)
number literal (differing) :
null & undefined x 35,193,209 ops/sec ±0.52% (95 runs sampled)
date (differing) x 22,800,127 ops/sec ±0.87% (91 runs sampled)
error x 10,237,955 ops/sec ±0.54% (91 runs sampled)
regex ctor (differing) x 22,276,672 ops/sec ±0.69% (88 runs sampled)
string ctor (differing) x 9,576,583 ops/sec ±0.57% (91 runs sampled)
weakmap x 12,350,834 ops/sec ±1.08% (87 runs sampled)
weakset x 11,459,314 ops/sec ±0.69% (90 runs sampled)
promise x 13,137,372 ops/sec ±0.65% (90 runs sampled) |
I'd gladly accept PRs to improve it. (I tried to optimize the obvious On Sun, Sep 11, 2016, 23:33 crazyhuggins notifications@github.com wrote:
|
Hi. I asked my firend about it earlier if he would help out, and he told me on Skype that he was looking at your requirments. So he would need to fire up Karma too. I guess he either send a PR or invite you to a repo once he is done so you could cherry pick. |
@isiahmeadows Hi! I'm looking into your algorithm now, and I will come up with a improved solution within a few days I guess. I'm new to open source and little scared to dive into others code without any knowledge, so I will just put up a repo and let you take it from there if you like my solution. Just now I have everything covered, but I guess massive use of Another issue is - if this is going to run in browser - WeakMap and WeakSet. Both of this two has issues with IE11, and need some work to patch it. Also same with Promises. There are loads of different promise libraries, and feature detect native ES6 Promises is easy, but not allways you can rely on It's also hilarious how Lodash claim to have best performance, but it's turn out it's one of the worst on performance if you benchmark isEqual. Left now is Map() and Set(). I guess I can inherit some of your great work? Outstanding good job with this library!! 👍 |
@zubuzon I understand the feeling of uncertainty when you're just getting started with open source. We've all been there at some point. And in case you're wondering, the best way to learn about how code works is just to play with it. 😄
It shouldn't have much of a performance impact beyond what you would normally expect of type checking operators (beyond
It does support browsers (tested in Firefox and Chrome), and it's also tested in legacy engines (all the way to Node 0.10, which has zero ES6 support, and PhantomJS 1.x, running an ancient WebKit version used in Safari 5). So WeakMaps and WeakSets (as well as Maps and Sets) aren't guaranteed to exist. The Promises used everywhere are actually Bluebird promises, not native ones.
The performance focus IIRC was on the iteration functions like
Thank you! 😄 |
Oh, all you need is to do this:
On Mon, Sep 12, 2016, 08:58 zubuzon notifications@github.com wrote:
|
That's generally the workflow with most GitHub projects. (Larger projects On Mon, Sep 12, 2016, 16:44 Isiah Meadows impinball@gmail.com wrote:
|
Hi. I will just experiment a little regarding this. However you can speed up the regEx and date comparison checks very easy refactor function deepEqual() {
// do primitive check directly here
return objMatch(matcher, deepEqual, a, b); // then this...
}
function objMatch(matcher, deepEqual, a, b) {}
.../ No need for strictIs etc now. Only use deepEqual That gave me improved performance. And for the keys you are checking, just call the same deepEqual function again. I'm still experimenting on this, and I found issues with your Set() implementation. Set() can actually have circular references, but in your code this will break. var set1 = new Set;
var set2 = new Set;
set1.add(set1);
set2.add(set2);
// set1 and set2 -> true
set1.add(1);
set2.add(2);
// set1 and set2 -> false And I'm not sure about transitive equivalence for circular references. In fact there are various stuff I have noticed so far, but I will look into it. Just now I got a unforeseen meeting with a client so I have to deal with that first. |
Another issue is your a !== 0 || 1 / a === 1 / b; // original code This will anyway be fixed if you In first place I'm going to put together some code that demonstrate all this, and we can discuss it from there before I port the code to Thallium (if you like my solutions) Now I'm heading out for my meeting. |
The double check for circular references is actually to check that each As for I do suspect the primitive issue is because I abstracted too much (no On Mon, Sep 12, 2016, 20:37 zubuzon notifications@github.com wrote:
|
I'm in the car now, but i can look into your CLI soon as this is settled. However. Im not sure if we are following the ECMA standards here? If so in On Sep 13, 2016 8:56 AM, "Isiah Meadows" notifications@github.com wrote:
|
Yes, that's the case for primitives of the same type. I'll try to find time (By the way, I don't differentiate positive and negative 0.) On Mon, Sep 12, 2016, 21:09 zubuzon notifications@github.com wrote:
|
I will put up a demo repo for this soon as I get home. I got it all working I probably would need to run all tests through karma as well. On Sep 13, 2016 9:13 AM, "Isiah Meadows" notifications@github.com wrote:
|
One quick thing to mention before I'm gone for awhile. I did a totaly rewrite of your code in my experiment, but copied your tests. All your tests passes 100%. In fact I have around 100 tests running. You will see it later. |
@isiahmeadows I added a repo without the source for now, here: https://github.com/zubuzon/booxi |
@isiahmeadows My experiment code is now uploaded, so I will just play around with it to find the perfect combination with bits and bytes and then try to implement it on Thallium as a PR. I linked to the repo above, and sent you a invite for access rights as well. update You are missing a lot of features, that now gives me headache. See this as one example: https://github.com/zubuzon/booxi/issues/1 Also your update again Sorry to say. I couldn't figure out your Map() implementation, so I made a simpler solution and made sure all your Map() tests works. And circular references for Map() also fixed 👍 Your Map() performance
map x 381,333 ops/sec ±0.46% (91 runs sampled)
My experiment
map x 594,083 ops/sec ±1.46% (91 runs sampled) Anyway. My main goal now will be to add support for EVERYTHING, then performance and back port this to Thallium. Status
Both Loose and Strict mode works as well, but didn't figure out the difference between them and Match mode yet. But should work too. Not sure if I should continue on this? It will probably be rejected, and my head isn't with me now :) |
@isiahmeadows I worked a little more on this now, and have almost implemented all features you was missing, and the performance seems to be okay. For generators I get this perf: generator func (differing) x 13,905,669 ops/sec ±0.76% (89 runs sampled) Still things has to be moved around a little to gain even better performance, and fix the Key issue, but after that I think this is good to go? |
I'm done! @crazyhuggins you see my prototype / experimental solution here: https://github.com/zubuzon/booxi There are little perf loss now, but still faster then current implementation. And there is a few issues yet to solve, but this is a prototype, and I'm not even sure if @isiahmeadows want me to back port this solution to Thallium. |
Feel free to go ahead and do it yourself if you like! Recent family issues (Keep in mind this is more of a pet project of mine, something I've been On Tue, Sep 13, 2016, 10:02 zubuzon notifications@github.com wrote:
|
Sorry to hear about your personal issues :( I have client meetings the next days but i will try to squeeze in some time No worries I will try my best to stay true to your original code, but Take care of your family1 On Sep 13, 2016 10:40 PM, "Isiah Meadows" notifications@github.com wrote:
|
Seems like I don't get any time to continue on this the next days. And I guess I would need your opinion regarding my experiment if I'm thinking correctly before event try to port it and send a PR. |
First, I may not be able to actually do anything other than look at it and Second, the Third, I am very strict on the style and tests, but it helps me manage the On Tue, Sep 13, 2016, 19:31 zubuzon notifications@github.com wrote:
|
@zubuzon If you'd like, you can send a PR. Note that Thallium is in pure ES5, not TypeScript. |
@isiahmeadows I had plans to port my work over to Thallium and open a PR soon as I got time. I know it's pure ES5. Just me that allways using TS so I easier can see if there are misstakes in the code. I did some interesting findings. If you create a new Matcher instance each time you use your code, and pass it through from function from function you will loose simple object comparison performance with around 25%. However. If you do use same solution as NodeJS, your perf will be much better, but you are stuck on large arrays and objects. Not ideal either. The ideal solution to get it working for large data sets, and have a OK performance is to have a set of various classes that inherit from each other, and created if needed. matcher || ( matcher = new matcher); is the best solution I found. Next is use of Just mention it what I found out. |
Here is an not public demo I did with class inheritance to gain better performance so you may understand the huge difference in perf. I'm all gone now. Need to take care of clients :( locale x 75,765,397 ops/sec ±0.74% (94 runs sampled)
booxi x 44,651,586 ops/sec ±0.55% (94 runs sampled) |
Yeah...this is kind of large for a JS-only project, anyways (if core was any more complex, I'd consider TypeScript, anyways).
Using shared data should make a significant difference. I used an object at first for easier debugging in the initial work, but converting that to using a global closure would be almost trivial.
I may do that for objects (not allocate the array for primitives, and unallocate after completion).
I'm not that worried about Also, here's why just checking for const foo1 = /foo/
const foo2 = /foo/
foo1.bar = 1
t.match(foo1, foo2) // should be false Although I can sidestep that by doing a quick, cheap function isEmpty(obj) {
for (var key in obj) if ({}.hasOwnProperty.call(obj, key)) return true
return false
} I eventually plan on pulling this out of core, anyways. It most definitely doesn't belong here in the long run. |
For your regExp example you could simply do Object.is(a, b); I will try to squeeze in some time sunday and look into this again. |
@isiahmeadows I removed the Booxi repo. And working on something new regarding this now that will replace the Booxi code. |
@zubuzon Note that parts of it may become moot, since I'm now using actual CPU profiling tools here. I've finally got time to dedicate to this. Expect a new commit or three soon. |
@isiahmeadows Sounds good. I'm trying to learn your code style now and figure out how your module is working so I probably can help out in my spare time. If you want. In case you wonder. I'm a freelancer now but worked 8 years in a company you are using everyday. I'm not doing any marketing here so I keep the name on the company for myself :) Wich part are you optimizing, and how? I noticed that object equality checks are terrible slow in most of all similiar implementations. And be aware that And I also found that +0 and -0 is actualy true after the specs in ECMA 2016 and upcoming ECMA 2017. I just mention this in case you want to normalize this behaviour to follow the specs. You may also allow equality checks of DOM nodes if you plan this to work in a browser? The Jasmine implementation of deepEqual does this. And so does Lodash, but Lodash does it very very slow it seems. |
And if you want to go all the way and actually create a ultimate solution, you may also consider comparing style objects in browsers. |
@zubuzon
I'm mainly trying to optimize the cases of plain objects (Thallium's own tests use them extensively), primitives, things like And yes, I'm aware IE11 has no support for those (spoiler alert: Node 0.10, which this does support, doesn't, either).
You can't iterate WeakSets, either, so I don't even bother. Yes, Also, WeakMaps and WeakSets only accept objects and functions, not even Symbols (numbers are often unboxed, anyways, so they can't be used reliably). It mainly exists for private data and tagging.
I understand, but I still plan on keeping
That's not going into core. I've already ditched loose equality-based matching because it slowed things down by complicating everything. |
If you use This is just the same as (Symbol(), Symbol()); and var symbol = Symbol()
(symbol, symbol); Only the last example should return true. |
@zubuzon Yes, but
The reason why I'm not going to check for WeakMaps or WeakSets in the deep equality algorithm is because the data within it is inaccessible by design (by TC39) and the nature of deep equality is to check structure, not referential equality. And if you need to check actual referential equality, Sorry if I'm coming across as a little blunt, but I have to draw the line on that one. |
I understand. Will be interesting to see your new changes. On Sep 17, 2016 11:43 AM, "Isiah Meadows" notifications@github.com wrote:
|
Did you finish a refactoring or how it goes? On Sep 17, 2016 12:05, "Kenny Flashlight" kflash123@gmail.com wrote:
|
@isiahmeadows You seem to like Mithril. Let me know if you want to help out - in progress - fastest ui interface framework that is 90% faster then Mithril or any other frameworks |
@zubuzon Thanks, but not too interested ATM. (Due to a flurry of things going on recently, I've barely had time to even work on this.) I'm personally working off-and-on on another vdom-style framework that is supposed to even support animations (fast with real-time requirements), but I'm keeping it closed until I have something usable to start with. Most of the complexity is from the real-time requirements (I need guaranteed constant-time insertions and deletions, as well as linear iterations) and because the framework lets components control their own rendering and updates ( I've got a secret gist explaining this, but I plan to open it up as soon as I have the concepts solidified. What's mainly holding me back, though, is that the API is so coupled to the implementation details in the case of components that render themselves, much like how React's custom renderers also have to have at least some knowledge of the internal data structures.
It's going well. It's fun using IRHydra (and less so |
Actually what i mentioned you cant call it a virtual dom. It's a virtual Instant 60 FPS at 50% and 51 FPS at 100%. Sounds interesting your RL logic and animation plans. Is it js animation or @isiahmeadows Main issue is - if you follow React - that you would need to work with objects and iterate through an object before e.g. setting CSS values { Get the styles inside the plain object, and iterate through the 'styles' object is expensive. I'm not doing that. I skip it, and set the new value directly. When do you plan to open up your gist? I must say i like your skills! Mostly CPU profiling isnt helpfull. Irhydra is the best tool i guess. I will continue to keep and eye on your work. Cheers On Sep 19, 2016 09:15, "Isiah Meadows" notifications@github.com wrote:
|
@zubuzon
True that static generation is always faster.
Mine will be JS animations (including
Thanks. And I don't really have a set time frame for it other than when I feel it's ready. It's just a spare time project with little actual importance.
It largely depends. If you're working at a higher level, like at the application level or framework level (e.g. a rendering algorithm), profiling is very useful, especially when using Chrome's CPU profiler and flame graphs. If the issue is I/O (e.g. frequent file system access), there's specialized tools for that. At a lower level (e.g. deep equality testing), you'll need to inspect the branches themselves, which bytecode inspectors like IRHydra are for. (My biggest issue ATM is that I can't get the |
I simply do this: Var template= createTemplate(function(x , y) { Render(template(23, 45), document.body); If a ball is jumping. Only x and y is updated. And each template can be Hobby project. Too many clients so have not so much time On Sep 19, 2016 10:09, "Isiah Meadows" notifications@github.com wrote:
|
@zubuzon I've pushed a new commit about this, and filed #11 in response. |
I can try to look at it within a few hours On Sep 21, 2016 18:36, "Isiah Meadows" notifications@github.com wrote:
|
I looked at your improved matcher and it looks good. However you are And i noticed almost identical code for primitives. Simplify? On Sep 21, 2016 18:53, "Kenny Flashlight" kflash123@gmail.com wrote:
|
I'll consider those, but I'll note that I can't compare promises, because I Oh, and the duplication was on purpose. It actually speed the thing up On Wed, Sep 21, 2016, 09:38 zubuzon notifications@github.com wrote:
|
Closing, since the original concern was already addressed. |
Hi. Great work with this project! I really like it 👍
However I was looking at substack repo and discovered this: inspect-js/node-deep-equal#42
The text was updated successfully, but these errors were encountered: