[SERVER-446] finish v8 engine Created: 29/Nov/09 Updated: 12/Jul/16 Resolved: 02/Feb/10 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | JavaScript |
| Affects Version/s: | None |
| Fix Version/s: | 1.3.2 |
| Type: | New Feature | Priority: | Major - P3 |
| Reporter: | Eliot Horowitz (Inactive) | Assignee: | Aaron Staple |
| Resolution: | Done | Votes: | 7 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Participants: | |||||||||||||
| Description |
|
scons --usev8 builds all binaries with v8 instead of spider monkey. |
| Comments |
| Comment by ZedroS [ 06/Apr/11 ] |
|
thanks eliot |
| Comment by Eliot Horowitz (Inactive) [ 04/Apr/11 ] |
|
@Zedross - it is finished. if you want track when we are going to switch, watch |
| Comment by ZedroS [ 04/Apr/11 ] |
|
for when is it due ? |
| Comment by Eliot Horowitz (Inactive) [ 07/Feb/10 ] |
|
passes all tests and is in bb |
| Comment by auto [ 02/Feb/10 ] |
|
Author: {'login': 'astaple', 'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 02/Feb/10 ] |
|
Author: {'login': 'astaple', 'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 02/Feb/10 ] |
|
Author: {'login': 'astaple', 'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 29/Jan/10 ] |
|
now a v8 64-bit linux buildbot slave |
| Comment by Aaron Staple [ 21/Jan/10 ] |
|
Would be cool to have a v8 buildbot. I tried running the js tests yesterday and saw a failure, so there's already been a regression in the last few weeks. |
| Comment by Aaron Staple [ 29/Dec/09 ] |
|
Okay, I believe I fixed the scope reuse issues with the above commits. |
| Comment by auto [ 29/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 29/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 28/Dec/09 ] |
|
Yes - that's fine for now. |
| Comment by Aaron Staple [ 28/Dec/09 ] |
|
Ok, I'm trying to make it work and I'd like to know what the semantics of a Scope reset() are. It looks like in the sm engine reset() doesn't actually do anything: void reset(){ Is that intentional? Is it ok to leave user data sitting in a scope in my v8 implementation? |
| Comment by Eliot Horowitz (Inactive) [ 28/Dec/09 ] |
|
Currently its definitely expensive because it has to re-run all the js-helpers. |
| Comment by Aaron Staple [ 28/Dec/09 ] |
|
I haven't investigated the details, but v8 expects v8::Context's to be entered in a hierarchical fashion and internally maintains a stack of the active contexts. When a v8::Context is exited, v8 automatically enters the next context in its stack. When we reuse V8Scope's, we can't completely reset the v8::Context's stored within them, and v8 asserts when the ordering relationships are not as expected. I'm sure there's some way of making this work, I just wanted to make sure it's worth the time. I assume scope caches were implemented because creating a scope is expensive in spidermonkey. Do we know the same is expensive in v8? |
| Comment by Eliot Horowitz (Inactive) [ 28/Dec/09 ] |
|
The scope cache is a very big deal for a lot of things. What's the problem? For using v8 for testing it ok, but to use v8 as a spidermonkey replacement we need the caches to work. |
| Comment by Aaron Staple [ 28/Dec/09 ] |
|
Is it ok if I disable the scope cache for v8? The current scope cache implementation doesn't work well with v8's management of entered/exited contexts. |
| Comment by Eliot Horowitz (Inactive) [ 23/Dec/09 ] |
|
Need to get running in a buildbot environment. |
| Comment by auto [ 22/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Aaron Staple [ 22/Dec/09 ] |
|
Updating my v8 code fixed the memory problem - must have gotten a bad rev. earlier. |
| Comment by Eliot Horowitz (Inactive) [ 22/Dec/09 ] |
|
Is it running out of v8 memory or total memory? Is the v8 memory configurable? |
| Comment by Aaron Staple [ 22/Dec/09 ] |
|
Last failure in the basic tests is that in mr1.js the db process runs out of memory with v8 engine. I can try to investigate myself, but please let me know first if you have any thoughts about the cause of this. |
| Comment by auto [ 22/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 22/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 22/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 22/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Aaron Staple [ 21/Dec/09 ] |
|
Thanks for letting me know - will do |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 21/Dec/09 ] |
|
That's fine - just make sure to update the website docs with a note about 1.3 |
| Comment by Aaron Staple [ 21/Dec/09 ] |
|
Gotcha. Is it ok if we change the syntax to Object.bsonsize( x ) ? That would be much easier to implement in v8 unless we don't care about polluting Object.prototype with an extra key. |
| Comment by Eliot Horowitz (Inactive) [ 21/Dec/09 ] |
|
we definitely need that and cannot get rid of it |
| Comment by Aaron Staple [ 21/Dec/09 ] |
|
Also getting rid of the bsonsize() function – it's only used in one test, and can use the datasize command there instead. If it's a problem, just let me know. |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Aaron Staple [ 21/Dec/09 ] |
|
v8 reorders fields with numeric names In v8: ; { "4" : 2, "x" : 1 }while in sm: I'm assuming this isn't a big deal for us. I'm just going to make the tests more flexible where they are failing currently. |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 21/Dec/09 ] |
|
lets make v8 work like spider monkey for now |
| Comment by Aaron Staple [ 21/Dec/09 ] |
|
An alternative solution would be to have both engines just drop undefined values. |
| Comment by Aaron Staple [ 21/Dec/09 ] |
|
It appears that when spidermonkey is invoked to return a value from a native c++ callback, values which the callback returned as 'undefined' type are converted to 'null' type. As a result, undefined values are converted to null when the db is queried. This conversion is not performed by v8, and as a result one (or more) of the js tests don't pass, producing obscure output. Should I make v8 perform the same conversion undefined->null? Alternatively I could just fix the test(s) individually. |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Aaron Staple [ 21/Dec/09 ] |
|
I'm implementing a js version of keySet and adding it to Object rather than Object.prototype to avoid having a new and confusing 'keySet' key in all objects when doing manual iteration. This requires that keySet be called explicitly via Object.keySet() rather than simply doing obj.keySet(). Hopefully this new interface is ok. |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 21/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 16/Dec/09 ] |
|
Yeah - probably can do something like that actually. |
| Comment by Aaron Staple [ 16/Dec/09 ] |
|
Would something like this work for grabbing the keys that would be in keyset? for( i in z ) { if ( !( i in z._proto_ ) || z[i] != z._proto_[i] ) { print( i ); }} |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 16/Dec/09 ] |
|
I believe its impossible. |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Aaron Staple [ 16/Dec/09 ] |
|
It looks like the keySet() function is hardcoded in the spidermonkey engine currently. Would it make sense to write this in javascript instead? |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 16/Dec/09 ] |
|
Yeah - just don't worry about write protection for arrays |
| Comment by Aaron Staple [ 16/Dec/09 ] |
|
It looks like it's impossible in v8 to implement access interceptors on a value which v8 represents internally as an array. Right now I'm using access interceptors to implement write protection, but to do so I am converting BSON arrays to v8 objects instead of v8 arrays (and then back into BSON arrays round-trip). However, with this implementation we can't treat the BSON arrays as arrays in js code (for example forEach() won't work, since v8 thinks it's dealing with a non-array object). I could conceivably use v8 arrays instead of v8 objects in cases where write protection isn't required, but then we'd have two different array formats and that could be confusing to a js programmer. Inheriting from Array doesn't work correctly in v8, so that isn't an option for getting around this problem. I could post a question on the v8 user group, but I may not get any helpful feedback. What should I do? Just disable write protection for arrays? |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 16/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 15/Dec/09 ] |
|
i think this is ok. |
| Comment by Aaron Staple [ 15/Dec/09 ] |
|
Right now in sm, if a double that could be represented as an int is moved to sm and back, it remains a double. Right now in v8, in the same situation, the double is returned as an int without loss of information. Is this ok? I'm just asking because the jstests don't check this behavior in TypeConservation though they test several other cases (I added a commented out - |
| Comment by auto [ 15/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 15/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by auto [ 15/Dec/09 ] |
|
Author: {'name': 'Aaron', 'email': 'aaron@10gen.com'}Message: |
| Comment by Eliot Horowitz (Inactive) [ 09/Dec/09 ] |
|
No - that APi is not really usable, and I think v8 and safari are neck and neck for speed. |
| Comment by Alexander Kellett [ 09/Dec/09 ] |
|
AFAIU v8 is still quite a bit slower than the C++ based engine in Safari. Have you considered using that instead? |