Add support for installing self-hosting intrinsics via JSAPI

ASSIGNED
Assigned to
(NeedInfo from)

Status

()

Core
JavaScript Engine
ASSIGNED
2 years ago
2 years ago

People

(Reporter: till, Assigned: till, NeedInfo)

Tracking

(Depends on: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
This is what we talked about, and should be very straight-forward.
(Assignee)

Comment 1

2 years ago
Created attachment 8690023 [details] [diff] [review]
Add support for installing self-hosting intrinsics via JSAPI
Attachment #8690023 - Flags: review?(efaustbmo)
Attachment #8690023 - Flags: feedback?(mwu)
Attachment #8690023 - Flags: feedback?(bzbarsky)
Comment on attachment 8690023 [details] [diff] [review]
Add support for installing self-hosting intrinsics via JSAPI

Looks fine, for stuff we don't need to get inlined.
Attachment #8690023 - Flags: feedback?(bzbarsky) → feedback+

Comment 3

2 years ago
Comment on attachment 8690023 [details] [diff] [review]
Add support for installing self-hosting intrinsics via JSAPI

Review of attachment 8690023 [details] [diff] [review]:
-----------------------------------------------------------------

Looks about as I expected on the JSAPI side.
Attachment #8690023 - Flags: feedback?(mwu) → feedback+

Comment 4

2 years ago
Comment on attachment 8690023 [details] [diff] [review]
Add support for installing self-hosting intrinsics via JSAPI

Review of attachment 8690023 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/SelfHosting.cpp
@@ +1887,5 @@
> +bool
> +JSRuntime::addSelfHostingIntrinsics(const JSFunctionSpec* intrinsicFunctions)
> +{
> +    MOZ_ASSERT(hasContexts());
> +    MOZ_ASSERT(selfHostingGlobal_);

These asserts will cover it, but can we make it more explicit, like we did with the strings? !selfHostingInitialized, or whatever that check was?

Maybe I'm misremembering, but I thought we had something more obviously explicit there.
Attachment #8690023 - Flags: review?(efaustbmo) → review+
(Assignee)

Comment 5

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2231eb3fd856f25cc4cf05f81d0195bcc6527750
Bug 1226551 - Add support for installing self-hosting intrinsics via JSAPI. r=efaust, f=bz,mwu
(Assignee)

Comment 7

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5966ace0c93292c6a2dbca3eeedcbaa9b2dae701
Bug 1226551 - Add support for installing self-hosting intrinsics via JSAPI. r=efaust, f=bz,mwu
Backed out for Linux x64 opt Valgrind bustage together with bug 1224722: https://hg.mozilla.org/integration/mozilla-inbound/rev/496bd6468e61

https://treeherder.mozilla.org/logviewer.html#?job_id=17903617&repo=mozilla-inbound

==12218== LEAK SUMMARY:
==12218==    definitely lost: 0 bytes in 0 blocks
==12218==    indirectly lost: 0 bytes in 0 blocks
==12218==      possibly lost: 10,181 bytes in 188 blocks
==12218==    still reachable: 168,376 bytes in 537 blocks
==12218==         suppressed: 48,176 bytes in 43 blocks
==12218== Reachable blocks (those to which a pointer was found) are not shown.
==12218== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==12218==
==12218== ERROR SUMMARY: 42 errors from 42 contexts (suppressed: 7 from 7)
Flags: needinfo?(till)
You need to log in before you can comment on or make changes to this bug.