-
Notifications
You must be signed in to change notification settings - Fork 11
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
Hyrule isn't using bytecode files #42
Comments
Is it byte-compiled? Are you loading the bytecode instead of recompiling the source every time? |
How do I check if it is? |
Check the |
Well |
What about bytecode for the individual source files, such as |
So this is what I have in the directory:
|
The dates modified are bogus and |
Hah! Yeah, those dates are ridiculous. I'm using nixos, and this is from a custom environment. Every other module seems to work fine, though, interestingly enough. |
To me that smells like Hy considering the |
Yes, belatedly, I would add that a quick thing you can try is to |
Ah; should I touch all of them? Also, it's there a way to generate the bytecode before I import the module for the first time? That way I can have the install instructions generate them with the correct dates and times. |
Yes.
One thing at a time. Let's see if we can figure out your slowdown problem first. |
Right, right; I'll get right on it. |
Nope; didn't seem to fix it. I touched all the files, updated their timestamps, and tried again with the |
Okay, try updating Hy and Hyrule and then running your program with |
Er, quick error with the master branches of
|
Wait, no, sorry; my fault. |
@Kodiologist: Actually, since I'm using the master branch for these tests, how do decorators work now? I saw your comment, but I still don't quite understand it. |
And how do you define a tuple, now, as opposed to the previous |
Alright; here's the final result:
|
Yeah, so it's as I guessed, the bytecode isn't being installed or isn't being read. I can replicate it on my machine. Hy has the same problem, so, no big surprise, I guess. |
Makes sense; is it an OS issue, or a |
I think it's an issue on our end of how the installation works. We need to get |
Ah; that's unfortunate. If you would like me to run any more tests, just let me know; |
Doing some initial digging into this as part of a larger project revamping Hy's import system; Potential solutions:
I can look into the first two to see if there's an easy way to do either of those in the setup.py process. |
We already have reading and writing bytecode working in common cases, including the management of the timestamp stored in the bytecode, because we're letting Python's import system handle it. Are you sure mismatching timestamps is the problem here? |
In the nix environment, it is, since the source files all have timestamps reset to 1970. When python tries to load the pyc, it sees the mismatch between the timestamp in the pyc data and the mtime of the source, so it reloads the file (triggering recompilation). |
I see. Well, one can reproduce the problem in Ubuntu, and presumably other conventional Unices, so let's focus on that first. Then maybe NixOS's package manager can deal with its own problems. |
Do you have reproduction steps for Ubuntu? I can't reproduce this using virtualenvs |
I can't reproduce it in a virtualenv, either, only for system-wide installation. Presumably this is because of permissions: my normal user account can't save a bytecode file to the system-wide Python directory. |
With the master branches for both |
Oh shoot, I didn't mean to close this. Sunjay used the magic word "fix" that triggered a closure. I didn't even know that worked cross-repository.
No, it's because no changes have been made to Hyrule to make it precompile and install its own bytecode. |
Hah! Fair enough. I guess I'll wait for the update, then. |
@shadowrylander if you have some time before we bake the new changes into 1.0a5 (or 0.21.0?), can you confirm that the combination of hylang/hy#2309 and #46 work in your nix setup? |
Will do; I'll report back in a bit! |
Okay, while I can disable the following test, is there any way to fix it?
Otherwise, @scauligi, it works! In case you use with builtins; let
pkgs = import (fetchGit { url = "https://github.com/shadowrylander/nixpkgs"; ref = "j"; }) {};
hy = pkgs.python310Packages.hy.overridePythonAttrs (prev: {
src = fetchGit { url = "https://github.com/scauligi/hy"; ref = "the_wheel_deal"; };
# src = fetchGit { url = "https://github.com/hylang/hy"; ref = "master"; };
postPatch = ''substituteInPlace setup.py --replace "\"funcparserlib ~= 1.0\"," ""'';
disabledTests = [ "test_bin" "test_hy2py" ];
});
hyrule = pkgs.python310Packages.hyrule.overridePythonAttrs (prev: {
src = fetchGit { url = "https://github.com/scauligi/hyrule"; ref = "precompile_hy"; };
# src = fetchGit { url = "https://github.com/hylang/hyrule"; ref = "master"; };
postPatch = ''substituteInPlace setup.py --replace "'hy @ git+https://github.com/scauligi/hy@the_wheel_deal#egg=hy'," ""'';
propagatedBuildInputs = [ hy ];
disabledTestPaths = [ "tests/test_slicing.hy" ];
});
in with pkgs; mkShell {
buildInputs = with python310Packages; [ (oreo.overridePythonAttrs (prev: {
src = ./.;
propagatedBuildInputs = (filter (p: ! elem p.pname [ "hy" "hyrule" ]) prev.propagatedBuildInputs) ++ [ hy hyrule ];
})) ];
# shellHook = "hy tests.hy; exit";
} |
Ah, one of the recent commits on master renamed
Heck yeah!
Awesome, thanks! I'm still a nix noob so this helps a ton. |
Ah; might need to collect the nix garbage, then. 😹 And if you ever need help with nix, don't hesitate to ask! I'm still learning as well, but I'll be glad to help! Thanks again to the both of you, @scauligi and @Kodiologist! Should I choose this issue now, or when both |
No prob! And no need to do anything with the issue, github should close it automatically once the PRs land. |
Got it; thanks again! |
Hello!
As in the title; I tried using the master versions as well, and the issue persists. Removing
hyrule
imports and requires from my module, while keeping all other modules, sped up the import of my module.Sorry if this isn't the place for this, as I don't know whether this counts as a bug or not.
Thank you kindly for the help!
The text was updated successfully, but these errors were encountered: