Closed Bug 461996 Opened 16 years ago Closed 8 months ago

JavaScript shell should link against JS shared library

Categories

(Core :: JavaScript Engine, enhancement)

x86
Linux
enhancement

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: jimb, Unassigned)

References

Details

Attachments

(2 files)

With the patch from 433083 applied, the JavaScript shell statically links all the SpiderMonkey object files, instead of linking dynamically against the SpiderMonkey shared library.  This is because the shell refers to js_InitJITStatsClass and jitstats_class, which are hidden symbols defined in jstracer.cpp ("hidden" meaning "visible within object files of the shared library, but not outside the shared library").

Either the interface to the statistics should be improved and made public, or the shell should not use it; either way, the shell should link against the shared library, not contain its own copies of all the library's object files.
In-person discussion (OMG!) says: just make the JIT stats stuff visible at the linker level, but not in headers.
Severity: normal → enhancement
A more palatable alternative would be to fix the build system so that we can produce both a static library and a shared library in the same Makefile, and then the shell can link to the static library.
I can see how that ability might be valuable, but the only reason static linking is relevant to this bug (that I know of) is that we have unresolved questions about what should be exported, and HIDDEN doesn't have its intended effect when using static libraries.  Using that behavior to avoid answering the interface question seems like a non sequitur.
While you should probably sort that out, having a js shell that's linked to a shared libjs will force users to set their library path appropriately, and that seems annoying.
Attachment #351234 - Flags: review? → review?(jim)
Comment on attachment 351234 [details] [diff] [review]
changes i needed to build a different js shell

I don't really feel qualified to review this patch, but I do have some comments, for whatever they're worth:

- Why is the external declaration for resolving_MatchEntry there?  I don't see any new references to it added by your patch, nor any references in js.cpp.

- Cleaning up the interface for the creation of jitstats_class seems like a clearly good thing --- even if we're not going to make it a public API.

- I somehow didn't run into any problems with JS_ObjIsNative, JS_ObjShape, JS_GetScriptAtom, JS_GetSourceNoteSpecName, and JS_GetNextSrcNote.  Why are they needed?  Are these good interfaces? If not, we shouldn't bother adding half-baked interfaces beyond those needed just to build.

- The changes for avoiding js_ScriptClass and js_FunctionClass are kind of kludgy, but they're better than breaking the interface.

- The change to SendSourceToJSDebugger is unrelated, isn't it? Shouldn't that just go in on its own?
Attachment #351234 - Flags: review?(jim)
while using this changeset to build jsdb on win32, i hit the others

most of my errors were linker errors, things that aren't JS_PUBLIC_API end up being mangled as C++ which causes serious issues.

iirc resolving_MatchEntry is supposed to be extern c by contract, not doing this results in warnings from compilers (or linker errors).
basically

typedef JSBool
(* JSDHashMatchEntry)(JSDHashTable *table, const JSDHashEntryHdr *entry,
                      const void *key);
is in an extern "C" block which means that the type you use for it is supposed to be C, solaris also tends to be very annoyed when you don't follow the rules.

the JS_* things are bits I hit while trying to link js.obj against js3250.dll for jsdb
(In reply to comment #2)
> A more palatable alternative would be to fix the build system so that we can
> produce both a static library and a shared library in the same Makefile, and
> then the shell can link to the static library.
This doesn't help my use case which is that each of js3250.dll and js.exe takes 11.9% of my total build time to link thus linking js.exe against js3250.dll would speed up my build by 13.5%.
Note that bug 501300 has some work that is incomplete, but may be useful.
(In reply to comment #8)
> This doesn't help my use case which is that each of js3250.dll and js.exe takes
> 11.9% of my total build time to link thus linking js.exe against js3250.dll
> would speed up my build by 13.5%.

Yeah, this sucks too. Since on Windows we compile with -GL, this means that the linker has to do all the hard work. Thus, every time you link the static library, the linker has to actually compile all that IL into object code. Since we're now linking: a) the DLL, b) js.exe, c) js-tests.exe, the linker gets to do that work 3 times (at > 30 seconds each on my machine).
Assignee: general → nobody
Severity: normal → S3
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: