Lay the source tree out in directories instead of one flat root - #67
Merged
Conversation
The repository root held the C++ sources, the OpenCL kernels, the Dockerfile, the build artifacts and the documentation side by side, so nothing about it said which file belonged to what. Sources now live in src/, kernels in kernels/, the shipping image in docker/, the recorded comparisons in bench/logs/, and make writes to build/ and bin/. The binary looks for its kernels next to itself and falls back to the working directory, so it can be started from anywhere: ./bin/profanity2.x64 from the repository root, from inside bin/, or by absolute path from an unrelated directory. Its compiled-kernel cache moves next to the binary for the same reason. make copies the kernels into bin/, which keeps that directory self-contained and lets the container image copy it as one. The benchmark tooling builds arbitrary revisions out of history and has to go on comparing a revision from before this change against one from after it, so bench/Dockerfile and bench/prepare-native.sh now take the build output from whichever of the two layouts a revision uses, and look for SpeedSample.cpp in both places. Co-authored-by: Cursor <cursoragent@cursor.com>
Asking git for a diff of SpeedSample.cpp between two revisions reports the move from the root of the tree into src/ as a change, so every comparison spanning the reorganization ended with "SpeedSample.cpp differs between the two revisions" and the warning that the two speeds come from different clocks. They do not: the file only moved. Each revision is now asked for the blob it records for the file, at either path, and the two blobs are compared. The image build already compared the extracted files with cmp and was never affected. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The repository root held the C++ sources, the OpenCL kernels, the Dockerfile, the build artifacts and the documentation side by side. Everything now has a directory:
Two things needed real changes rather than a move:
Finding the kernels.
profanity.cppread them from the working directory. It now looks next to the executable first, then inkernels/beside it, and falls back to the working directory, which is where every earlier release kept them. The compiled-kernel cache moves next to the binary for the same reason. So./bin/profanity2.x64works from the repository root, from insidebin/, and by absolute path from an unrelated directory.Comparing across the move.
bench/builds arbitrary revisions out of history, and has to go on comparing a revision from before this change against one from after it.bench/Dockerfileandbench/prepare-native.shnow take the build output from whichever layout a revision uses. The second commit fixes a false positive this introduced: agit diffofSpeedSample.cppreported the move itself as a change, so every comparison spanning the reorganization warned that the two revisions time a round differently. The timer is now compared by blob rather than by path.Test plan
makefrom clean buildsbin/profanity2.x64with no warnings, kernels copied next to itbin/, and by absolute path from/tmp; the cache lands inbin/in all three cases and nothing is left in the working directorymake -C testsbuilds, andtest_correctness_pr49.x64passes run both fromtests/and from the rootbench/prepare-native.sh 90c61e3 868be80builds both layouts and lays them out identically;timer.stateissame, as it should be for a file that only movedDispatcher.o,Mode.o,precomp.oandSpeedSample.oare byte-identical between the two builds, onlyprofanity.odiffers. A speed comparison on the Mac this was written on was thermally saturated and could not tell the two apart from noise, which is why the object files are the evidence heredocker/Dockerfile(no local Docker daemon to check it with)Made with Cursor